Encode all values based on declared type (#10549)

* ServerRpc encoding uses type data from the interface
* Beans encoded on the server use reflection based on declared type
* Remove row numbers to enable adding test without changing old indices
* Update test to send non-primitive map values

Change-Id: I0462b547cb7de252564b3569420b0b24cee4515f

3 files changed, 102 insertions, 75 deletions

diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index ff59a76832..00b0d75c6c 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -71,6 +71,7 @@ import com.vaadin.client.communication.RpcManager;
 import com.vaadin.client.communication.StateChangeEvent;
 import com.vaadin.client.extensions.AbstractExtensionConnector;
 import com.vaadin.client.metadata.ConnectorBundleLoader;
+import com.vaadin.client.metadata.Method;
 import com.vaadin.client.metadata.NoDataException;
 import com.vaadin.client.metadata.Property;
 import com.vaadin.client.metadata.Type;
@@ -2393,12 +2394,29 @@ public class ApplicationConnection {
                 invocationJson.set(2,
                         new JSONString(invocation.getMethodName()));
                 JSONArray paramJson = new JSONArray();
-                boolean restrictToInternalTypes = isLegacyVariableChange(invocation);
+
+                Type[] parameterTypes = null;
+                if (!isLegacyVariableChange(invocation)) {
+                    try {
+                        Type type = new Type(invocation.getInterfaceName(),
+                                null);
+                        Method method = type.getMethod(invocation
+                                .getMethodName());
+                        parameterTypes = method.getParameterTypes();
+                    } catch (NoDataException e) {
+                        throw new RuntimeException("No type data for "
+                                + invocation.toString(), e);
+                    }
+                }
+
                 for (int i = 0; i < invocation.getParameters().length; ++i) {
-                    // TODO non-static encoder? type registration?
-                    paramJson.set(i, JsonEncoder.encode(
-                            invocation.getParameters()[i],
-                            restrictToInternalTypes, this));
+                    // TODO non-static encoder?
+                    Type type = null;
+                    if (parameterTypes != null) {
+                        type = parameterTypes[i];
+                    }
+                    Object value = invocation.getParameters()[i];
+                    paramJson.set(i, JsonEncoder.encode(value, type, this));
                 }
                 invocationJson.set(3, paramJson);
                 reqJson.set(reqJson.size(), invocationJson);
diff --git a/client/src/com/vaadin/client/communication/JsonEncoder.java b/client/src/com/vaadin/client/communication/JsonEncoder.java
index 0756d6b069..f3262c0106 100644
--- a/client/src/com/vaadin/client/communication/JsonEncoder.java
+++ b/client/src/com/vaadin/client/communication/JsonEncoder.java
@@ -59,8 +59,8 @@ public class JsonEncoder {
      * @param connection
      * @return JSON representation of the value
      */
-    public static JSONValue encode(Object value,
-            boolean restrictToInternalTypes, ApplicationConnection connection) {
+    public static JSONValue encode(Object value, Type type,
+            ApplicationConnection connection) {
         if (null == value) {
             return JSONNull.getInstance();
         } else if (value instanceof JSONValue) {
@@ -80,55 +80,58 @@ public class JsonEncoder {
             return new JSONNumber((Byte) value);
         } else if (value instanceof Character) {
             return new JSONString(String.valueOf(value));
-        } else if (value instanceof Object[]) {
-            return encodeObjectArray((Object[]) value, restrictToInternalTypes,
-                    connection);
+        } else if (value instanceof Object[] && type == null) {
+            // Non-legacy arrays handed by generated serializer
+            return encodeLegacyObjectArray((Object[]) value, connection);
         } else if (value instanceof Enum) {
             return encodeEnum((Enum<?>) value, connection);
         } else if (value instanceof Map) {
-            return encodeMap((Map) value, restrictToInternalTypes, connection);
+            return encodeMap((Map) value, type, connection);
         } else if (value instanceof Connector) {
             Connector connector = (Connector) value;
             return new JSONString(connector.getConnectorId());
         } else if (value instanceof Collection) {
-            return encodeCollection((Collection) value,
-                    restrictToInternalTypes, connection);
+            return encodeCollection((Collection) value, type, connection);
         } else if (value instanceof UidlValue) {
             return encodeVariableChange((UidlValue) value, connection);
         } else {
+            // First see if there's a custom serializer
+            JSONSerializer<Object> serializer = null;
+            if (type != null) {
+                serializer = (JSONSerializer<Object>) type.findSerializer();
+                if (serializer != null) {
+                    return serializer.serialize(value, connection);
+                }
+            }
+
             String transportType = getTransportType(value);
             if (transportType != null) {
+                // Send the string value for remaining legacy types
                 return new JSONString(String.valueOf(value));
-            } else {
-                // Try to find a generated serializer object, class name is the
-                // type
-                Type type = new Type(value.getClass());
-
-                JSONSerializer<Object> serializer = (JSONSerializer<Object>) type
-                        .findSerializer();
-                if (serializer != null) {
-                    return serializer.serialize(value, connection);
-                } else {
-                    try {
-                        Collection<Property> properties = type.getProperties();
-
-                        JSONObject jsonObject = new JSONObject();
-                        for (Property property : properties) {
-                            Object propertyValue = property.getValue(value);
-                            JSONValue encodedPropertyValue = encode(
-                                    propertyValue, restrictToInternalTypes,
-                                    connection);
-                            jsonObject.put(property.getName(),
-                                    encodedPropertyValue);
-                        }
-                        return jsonObject;
-
-                    } catch (NoDataException e) {
-                        throw new RuntimeException("Can not encode "
-                                + type.getSignature(), e);
+            } else if (type != null) {
+                // And finally try using bean serialization logic
+                try {
+                    Collection<Property> properties = type.getProperties();
+
+                    JSONObject jsonObject = new JSONObject();
+                    for (Property property : properties) {
+                        Object propertyValue = property.getValue(value);
+                        Type propertyType = property.getType();
+                        JSONValue encodedPropertyValue = encode(propertyValue,
+                                propertyType, connection);
+                        jsonObject
+                                .put(property.getName(), encodedPropertyValue);
                     }
+                    return jsonObject;
+
+                } catch (NoDataException e) {
+                    throw new RuntimeException("Can not encode "
+                            + type.getSignature(), e);
                 }
 
+            } else {
+                throw new RuntimeException("Can't encode " + value.getClass()
+                        + " without type information");
             }
         }
     }
@@ -152,13 +155,13 @@ public class JsonEncoder {
                     + valueType);
         }
         jsonArray.set(0, new JSONString(transportType));
-        jsonArray.set(1, encode(value, true, connection));
+        jsonArray.set(1, encode(value, null, connection));
 
         return jsonArray;
     }
 
-    private static JSONValue encodeMap(Map<Object, Object> map,
-            boolean restrictToInternalTypes, ApplicationConnection connection) {
+    private static JSONValue encodeMap(Map<Object, Object> map, Type type,
+            ApplicationConnection connection) {
         /*
          * As we have no info about declared types, we instead select encoding
          * scheme based on actual type of first key. We can't do this if there's
@@ -171,26 +174,43 @@ public class JsonEncoder {
 
         Object firstKey = map.keySet().iterator().next();
         if (firstKey instanceof String) {
-            return encodeStringMap(map, restrictToInternalTypes, connection);
-        } else if (restrictToInternalTypes) {
+            return encodeStringMap(map, type, connection);
+        } else if (type == null) {
             throw new IllegalStateException(
                     "Only string keys supported for legacy maps");
         } else if (firstKey instanceof Connector) {
-            return encodeConnectorMap(map, connection);
+            return encodeConnectorMap(map, type, connection);
         } else {
-            return encodeObjectMap(map, connection);
+            return encodeObjectMap(map, type, connection);
+        }
+    }
+
+    private static JSONValue encodeChildValue(Object value,
+            Type collectionType, int typeIndex, ApplicationConnection connection) {
+        if (collectionType == null) {
+            return encode(new UidlValue(value), null, connection);
+        } else {
+            assert collectionType.getParameterTypes() != null
+                    && collectionType.getParameterTypes().length > typeIndex
+                    && collectionType.getParameterTypes()[typeIndex] != null : "Proper generics required for encoding child value, assertion failed for "
+                    + collectionType;
+            Type childType = collectionType.getParameterTypes()[typeIndex];
+            return encode(value, childType, connection);
         }
     }
 
     private static JSONValue encodeObjectMap(Map<Object, Object> map,
-            ApplicationConnection connection) {
+            Type type, ApplicationConnection connection) {
         JSONArray keys = new JSONArray();
         JSONArray values = new JSONArray();
+
+        assert type != null : "Should only be used for non-legacy types";
+
         for (Entry<?, ?> entry : map.entrySet()) {
-            // restrictToInternalTypes always false if we end up here
-            keys.set(keys.size(), encode(entry.getKey(), false, connection));
+            keys.set(keys.size(),
+                    encodeChildValue(entry.getKey(), type, 0, connection));
             values.set(values.size(),
-                    encode(entry.getValue(), false, connection));
+                    encodeChildValue(entry.getValue(), type, 1, connection));
         }
 
         JSONArray keysAndValues = new JSONArray();
@@ -201,14 +221,14 @@ public class JsonEncoder {
     }
 
     private static JSONValue encodeConnectorMap(Map<Object, Object> map,
-            ApplicationConnection connection) {
+            Type type, ApplicationConnection connection) {
         JSONObject jsonMap = new JSONObject();
 
         for (Entry<?, ?> entry : map.entrySet()) {
             Connector connector = (Connector) entry.getKey();
 
-            // restrictToInternalTypes always false if we end up here
-            JSONValue encodedValue = encode(entry.getValue(), false, connection);
+            JSONValue encodedValue = encodeChildValue(entry.getValue(), type,
+                    1, connection);
 
             jsonMap.put(connector.getConnectorId(), encodedValue);
         }
@@ -217,21 +237,14 @@ public class JsonEncoder {
     }
 
     private static JSONValue encodeStringMap(Map<Object, Object> map,
-            boolean restrictToInternalTypes, ApplicationConnection connection) {
+            Type type, ApplicationConnection connection) {
         JSONObject jsonMap = new JSONObject();
 
         for (Entry<?, ?> entry : map.entrySet()) {
             String key = (String) entry.getKey();
             Object value = entry.getValue();
 
-            if (restrictToInternalTypes) {
-                value = new UidlValue(value);
-            }
-
-            JSONValue encodedValue = encode(value, restrictToInternalTypes,
-                    connection);
-
-            jsonMap.put(key, encodedValue);
+            jsonMap.put(key, encodeChildValue(value, type, 1, connection));
         }
 
         return jsonMap;
@@ -242,28 +255,23 @@ public class JsonEncoder {
         return new JSONString(e.toString());
     }
 
-    private static JSONValue encodeObjectArray(Object[] array,
-            boolean restrictToInternalTypes, ApplicationConnection connection) {
+    private static JSONValue encodeLegacyObjectArray(Object[] array,
+            ApplicationConnection connection) {
         JSONArray jsonArray = new JSONArray();
         for (int i = 0; i < array.length; ++i) {
             // TODO handle object graph loops?
             Object value = array[i];
-            if (restrictToInternalTypes) {
-                value = new UidlValue(value);
-            }
-            jsonArray
-                    .set(i, encode(value, restrictToInternalTypes, connection));
+            jsonArray.set(i, encode(value, null, connection));
         }
         return jsonArray;
     }
 
-    private static JSONValue encodeCollection(Collection collection,
-            boolean restrictToInternalTypes, ApplicationConnection connection) {
+    private static JSONValue encodeCollection(Collection collection, Type type,
+            ApplicationConnection connection) {
         JSONArray jsonArray = new JSONArray();
         int idx = 0;
         for (Object o : collection) {
-            JSONValue encodedObject = encode(o, restrictToInternalTypes,
-                    connection);
+            JSONValue encodedObject = encodeChildValue(o, type, 0, connection);
             jsonArray.set(idx++, encodedObject);
         }
         if (collection instanceof Set) {
diff --git a/client/src/com/vaadin/client/communication/URLReference_Serializer.java b/client/src/com/vaadin/client/communication/URLReference_Serializer.java
index f004980e56..e455d4ee93 100644
--- a/client/src/com/vaadin/client/communication/URLReference_Serializer.java
+++ b/client/src/com/vaadin/client/communication/URLReference_Serializer.java
@@ -46,8 +46,9 @@ public class URLReference_Serializer implements JSONSerializer<URLReference> {
     public JSONValue serialize(URLReference value,
             ApplicationConnection connection) {
         JSONObject json = new JSONObject();
+        // No type info required for encoding a String...
         json.put(URL_FIELD,
-                JsonEncoder.encode(value.getURL(), true, connection));
+                JsonEncoder.encode(value.getURL(), null, connection));
         return json;
     }