readability enhancement and assert check for proxy object #3032

dubbo-common/src/main/java/org/apache/dubbo/common/utils/Assert.java

@@ -28,6 +28,11 @@ public static void notNull(Object obj, String message) {
         }
     }
 
+    public static void notEmptyString(String str,String message) {
+        if(StringUtils.isEmpty(str)) {
+            throw new IllegalArgumentException(message);
+        }
+    }
     public static void notNull(Object obj, RuntimeException exception) {
         if (obj == null) {
             throw exception;

dubbo-common/src/test/java/org/apache/dubbo/common/utils/AssertTest.java

@@ -20,6 +20,7 @@
 import org.junit.Test;
 
 import static org.apache.dubbo.common.utils.Assert.notNull;
+import static org.apache.dubbo.common.utils.Assert.notEmptyString;
 
 public class AssertTest {
     @Test(expected = IllegalArgumentException.class)
@@ -31,4 +32,28 @@ public void testNotNull1() throws Exception {
     public void testNotNull2() throws Exception {
         notNull(null, new IllegalStateException("null object"));
     }
+
+    @Test
+    public void testNotNullWhenInputNotNull1() {
+        notNull(new Object(),"null object");
+    }
+
+    @Test
+    public void testNotNullWhenInputNotNull2() {
+        notNull(new Object(),new IllegalStateException("null object"));
+    }
+    @Test(expected = IllegalArgumentException.class)
+    public void testNotNullString() {
+        notEmptyString(null,"Message can't be null");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNotEmptyString() {
+        notEmptyString("","Message can't be null or empty");
+    }
+
+    @Test
+    public void testNotNullNotEmptyString() {
+        notEmptyString("abcd","Message can'be null or empty");
+    }
 }

dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/model/ConsumerModel.java

@@ -16,6 +16,8 @@
  */
 package org.apache.dubbo.rpc.model;
 
+import org.apache.dubbo.common.utils.Assert;
+
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.IdentityHashMap;
@@ -33,18 +35,39 @@ public class ConsumerModel {
 
     private final Map<Method, ConsumerMethodModel> methodModels = new IdentityHashMap<Method, ConsumerMethodModel>();
 
-    public ConsumerModel(String serviceName, Class<?> serviceInterfaceClass, Object proxyObject, Method[] methods, Map<String, Object> attributes) {
+    /**
+     *  This constructor create an instance of ConsumerModel and passed objects should not be null.
+     *  If service name, service instance, proxy object,methods should not be null. If these are null
+     *  then this constructor will throw {@link IllegalArgumentException}
+     * @param serviceName Name of the service.
+     * @param serviceInterfaceClass Service interface class.
+     * @param proxyObject  Proxy object.
+     * @param methods Methods of service class
+     * @param attributes Attributes of methods.
+     */
+    public ConsumerModel(String serviceName
+            , Class<?> serviceInterfaceClass
+            , Object proxyObject
+            , Method[] methods
+            , Map<String, Object> attributes) {
+
+        Assert.notEmptyString(serviceName, "Service name can't be null or blank");
+        Assert.notNull(serviceInterfaceClass, "Service interface class can't null");
+        Assert.notNull(proxyObject, "Proxy object can't be null");
+        Assert.notNull(methods, "Methods can't be null");
+
         this.serviceName = serviceName;
         this.serviceInterfaceClass = serviceInterfaceClass;
         this.proxyObject = proxyObject;
-
-        if (proxyObject != null) {
-            for (Method method : methods) {
-                methodModels.put(method, new ConsumerMethodModel(method, attributes));
-            }
+        for (Method method : methods) {
+            methodModels.put(method, new ConsumerMethodModel(method, attributes));
         }
     }
 
+    /**
+     * Return the proxy object used by called while creating instance of ConsumerModel
+     * @return
+     */
     public Object getProxyObject() {
         return proxyObject;
     }

dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java

@@ -50,14 +50,19 @@ private static Method findMethod(List<ProviderMethodModel> methods, String metho
                                      Class<?>[] paramTypes) {
         for (ProviderMethodModel model : methods) {
             Method m = model.getMethod();
-            if (m.getName().equals(method) && isMatch(m.getParameterTypes(), args, paramTypes)) {
+            if (isMatch(m, args, paramTypes,method)) {
                 return m;
             }
         }
         return null;
     }
 
-    private static boolean isMatch(Class<?>[] types, List<Object> args, Class<?>[] paramClasses) {
+    private static boolean isMatch(Method method,List<Object> args, Class<?>[] paramClasses,String lookupMethodName) {
+        if(!method.getName().equals(lookupMethodName)) {
+            return false;
+        }
+
+        Class<?> types[]=method.getParameterTypes();
         if (types.length != args.size()) {
             return false;
         }

dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/telnet/ListTelnetHandler.java

@@ -34,7 +34,7 @@
 import static org.apache.dubbo.registry.support.ProviderConsumerRegTable.isRegistered;
 
 /**
- * ListTelnetHandler
+ * ListTelnetHandler handler list services and its methods details.
  */
 @Activate
 @Help(parameter = "[-l] [service]", summary = "List services and methods.", detail = "List services and methods.")
@@ -59,7 +59,7 @@ public String telnet(Channel channel, String message) {
             }
         } else {
             service = (String) channel.getAttribute(ChangeTelnetHandler.SERVICE_KEY);
-            if (service != null && service.length() > 0) {
+            if (StringUtils.isNotEmpty(service)) {
                 buf.append("Use default service ").append(service).append(".\r\n");
             }
         }
@@ -119,9 +119,7 @@ private void printSpecifiedService(String service, StringBuilder buf, boolean de
 
     private void printSpecifiedProvidedService(String service, StringBuilder buf, boolean detail) {
         for (ProviderModel provider : ApplicationModel.allProviderModels()) {
-            if (service.equalsIgnoreCase(provider.getServiceName())
-                    || service.equalsIgnoreCase(provider.getServiceInterfaceClass().getName())
-                    || service.equalsIgnoreCase(provider.getServiceInterfaceClass().getSimpleName())) {
+            if (isProviderMatched(service,provider)) {
                 buf.append(provider.getServiceName()).append(" (as provider):\r\n");
                 for (ProviderMethodModel method : provider.getAllMethods()) {
                     printMethod(method.getMethod(), buf, detail);
@@ -132,9 +130,7 @@ private void printSpecifiedProvidedService(String service, StringBuilder buf, bo
 
     private void printSpecifiedReferredService(String service, StringBuilder buf, boolean detail) {
         for (ConsumerModel consumer : ApplicationModel.allConsumerModels()) {
-            if (service.equalsIgnoreCase(consumer.getServiceName())
-                    || service.equalsIgnoreCase(consumer.getServiceInterfaceClass().getName())
-                    || service.equalsIgnoreCase(consumer.getServiceInterfaceClass().getSimpleName())) {
+            if (isConsumerMatcher(service,consumer)) {
                 buf.append(consumer.getServiceName()).append(" (as consumer):\r\n");
                 for (ConsumerMethodModel method : consumer.getAllMethods()) {
                     printMethod(method.getMethod(), buf, detail);
@@ -151,4 +147,16 @@ private void printMethod(Method method, StringBuilder buf, boolean detail) {
         }
         buf.append("\r\n");
     }
+
+    private boolean isProviderMatched(String service, ProviderModel provider) {
+        return service.equalsIgnoreCase(provider.getServiceName())
+                || service.equalsIgnoreCase(provider.getServiceInterfaceClass().getName())
+                || service.equalsIgnoreCase(provider.getServiceInterfaceClass().getSimpleName());
+    }
+
+    private boolean isConsumerMatcher(String service,ConsumerModel consumer) {
+        return service.equalsIgnoreCase(consumer.getServiceName())
+                || service.equalsIgnoreCase(consumer.getServiceInterfaceClass().getName())
+                || service.equalsIgnoreCase(consumer.getServiceInterfaceClass().getSimpleName());
+    }
 }