Skip to content

Commit

Permalink
Fix 3105 , make invoke command with Json string parameter without "cl…
Browse files Browse the repository at this point in the history 
…ass" key
  • Loading branch information
@LiZhenNet
LiZhenNet committed Jan 5, 2019
1 parent 2911b31 commit c833771
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 19 deletions.
25 changes: 10 additions & 15 deletions ...c-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.dubbo.rpc.protocol.dubbo.telnet;

import com.alibaba.fastjson.JSON;
import com.alibaba.fastjson.JSONObject;
import org.apache.dubbo.common.extension.Activate;
import org.apache.dubbo.common.utils.ReflectUtils;
import org.apache.dubbo.common.utils.StringUtils;
Expand All @@ -27,14 +29,10 @@
import org.apache.dubbo.rpc.model.ProviderMethodModel;
import org.apache.dubbo.rpc.model.ProviderModel;

import com.alibaba.fastjson.JSON;
import com.alibaba.fastjson.JSONObject;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;

import static org.apache.dubbo.common.utils.PojoUtils.realize;
import static org.apache.dubbo.rpc.RpcContext.getContext;
Expand All @@ -50,19 +48,19 @@ private static Method findMethod(List<ProviderMethodModel> methods, String metho
Class<?>[] paramTypes) {
for (ProviderMethodModel model : methods) {
Method m = model.getMethod();
if (isMatch(m, args, paramTypes,method)) {
if (isMatch(m, args, paramTypes, method)) {
return m;
}
}
return null;
}

private static boolean isMatch(Method method,List<Object> args, Class<?>[] paramClasses,String lookupMethodName) {
if(!method.getName().equals(lookupMethodName)) {
private static boolean isMatch(Method method, List<Object> args, Class<?>[] paramClasses, String lookupMethodName) {
if (!method.getName().equals(lookupMethodName)) {
return false;
}

Class<?> types[]=method.getParameterTypes();
Class<?> types[] = method.getParameterTypes();
if (types.length != args.size()) {
return false;
}
Expand Down Expand Up @@ -99,13 +97,10 @@ private static boolean isMatch(Method method,List<Object> args, Class<?>[] param
if (!ReflectUtils.isCompatible(type, arg)) {
return false;
}
} else if (arg instanceof Map) {

This comment has been minimized.

Sign in to view

Copy link
@zonghaishang

zonghaishang Jan 6, 2019

Member

I think the original code should not be deleted, the code should be added in advance.

else if (arg instanceof JSONObject) {
    try {
        ((JSONObject) arg).toJavaObject(type);
    } catch (Exception ignored) {
        return false;
    }
} else if (arg instanceof Map) {
    String name = (String) ((Map<?, ?>) arg).get("class");
    Class<?> cls = arg.getClass();
    if (name != null && name.length() > 0) {
        cls = ReflectUtils.forName(name);
    }
    if (!type.isAssignableFrom(cls)) {
        return false;
    }
}
  1. The class field is still valid, so it should not be deleted.
  2. Because some overloaded methods pass the class field more efficiently, avoiding a JsonObject.toJavaObject invoke

This comment has been minimized.

Sign in to view

Copy link
@LiZhenNet

LiZhenNet Jan 6, 2019

Author Contributor

you are right，I have change it and add a UT to test this case
String name = (String) ((Map<?, ?>) arg).get("class");
Class<?> cls = arg.getClass();
if (name != null && name.length() > 0) {
cls = ReflectUtils.forName(name);
}
if (!type.isAssignableFrom(cls)) {
} else if (arg instanceof JSONObject) {
try {
((JSONObject) arg).toJavaObject(type);
} catch (Exception ex) {
return false;
}
} else if (arg instanceof Collection) {
Expand Down
6 changes: 3 additions & 3 deletions ...-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/support/DemoServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public Type enumlength(Type... types) {
return Type.Lower;
return types[0];
}

public Type getType(Type type) {
return type;
}
Expand Down Expand Up @@ -109,12 +109,12 @@ public long add(int a, long b) {

@Override
public int getPerson(Person person) {
return 1;
return person.getAge();
}

@Override
public int getPerson(Person person1, Person perso2) {
return 2;
return person1.getAge() + perso2.getAge();
}

}
29 changes: 28 additions & 1 deletion ...bo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokerTelnetHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.apache.dubbo.rpc.protocol.dubbo.support.DemoService;
import org.apache.dubbo.rpc.protocol.dubbo.support.DemoServiceImpl;
import org.apache.dubbo.rpc.protocol.dubbo.support.ProtocolUtils;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -198,6 +197,34 @@ public void testInvokeAutoFindMethod() throws RemotingException {
assertTrue(result.contains("ok"));
}

@Test
public void testInvokeJsonParamMethod() throws RemotingException {
mockChannel = mock(Channel.class);
given(mockChannel.getAttribute("telnet.service")).willReturn(null);
given(mockChannel.getLocalAddress()).willReturn(NetUtils.toAddress("127.0.0.1:5555"));
given(mockChannel.getRemoteAddress()).willReturn(NetUtils.toAddress("127.0.0.1:20886"));

ProviderModel providerModel = new ProviderModel("org.apache.dubbo.rpc.protocol.dubbo.support.DemoService", new DemoServiceImpl(), DemoService.class);
ApplicationModel.initProviderModel("org.apache.dubbo.rpc.protocol.dubbo.support.DemoService", providerModel);
String param = "{\"name\":\"Dubbo\",\"age\":8}";
String result = invoke.telnet(mockChannel, "getPerson(" + param + ")");
assertTrue(result.contains("8"));
}

@Test
public void testInvokeMultiJsonParamMethod() throws RemotingException {
mockChannel = mock(Channel.class);
given(mockChannel.getAttribute("telnet.service")).willReturn(null);
given(mockChannel.getLocalAddress()).willReturn(NetUtils.toAddress("127.0.0.1:5555"));
given(mockChannel.getRemoteAddress()).willReturn(NetUtils.toAddress("127.0.0.1:20886"));

ProviderModel providerModel = new ProviderModel("org.apache.dubbo.rpc.protocol.dubbo.support.DemoService", new DemoServiceImpl(), DemoService.class);
ApplicationModel.initProviderModel("org.apache.dubbo.rpc.protocol.dubbo.support.DemoService", providerModel);
String param = "{\"name\":\"Dubbo\",\"age\":8},{\"name\":\"Apache\",\"age\":20}";
String result = invoke.telnet(mockChannel, "getPerson(" + param + ")");
assertTrue(result.contains("28"));
}

@Test
public void testMessageNull() throws RemotingException {
mockChannel = mock(Channel.class);
Expand Down

0 comments on commit c833771

Please sign in to comment.