]> source.dussan.org Git - vaadin-framework.git/commitdiff
Separate and improve JSON encoding/decoding of values (#8277, #8279).
authorHenri Sara <hesara@vaadin.com>
Fri, 20 Jan 2012 11:03:43 +0000 (13:03 +0200)
committerHenri Sara <hesara@vaadin.com>
Wed, 25 Jan 2012 11:49:13 +0000 (13:49 +0200)
src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
src/com/vaadin/terminal/gwt/client/Util.java
src/com/vaadin/terminal/gwt/client/communication/JsonEncoder.java [new file with mode: 0644]
src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java
src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java
src/com/vaadin/terminal/gwt/server/JsonDecoder.java [new file with mode: 0644]
src/com/vaadin/terminal/gwt/server/PaintableIdMapper.java [new file with mode: 0644]

index 6f7d6818f9b3aa758849d63744b1c6505b6807a9..88c1c389d447e9a038bb06667130bcb0add8dfc1 100644 (file)
@@ -38,6 +38,7 @@ import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.ApplicationConfiguration.ErrorMessage;
 import com.vaadin.terminal.gwt.client.RenderInformation.FloatSize;
 import com.vaadin.terminal.gwt.client.RenderInformation.Size;
+import com.vaadin.terminal.gwt.client.communication.JsonEncoder;
 import com.vaadin.terminal.gwt.client.communication.MethodInvocation;
 import com.vaadin.terminal.gwt.client.ui.Field;
 import com.vaadin.terminal.gwt.client.ui.VContextMenu;
@@ -80,9 +81,6 @@ public class ApplicationConnection {
 
     public static final char VAR_BURST_SEPARATOR = '\u001d';
 
-    @Deprecated
-    public static final char VAR_ARRAYITEM_SEPARATOR = '\u001c';
-
     public static final char VAR_ESCAPE_CHARACTER = '\u001b';
 
     public static final String UIDL_SECURITY_TOKEN_ID = "Vaadin-Security-Key";
@@ -1106,16 +1104,16 @@ public class ApplicationConnection {
     }-*/;
 
     private void addVariableToQueue(String paintableId, String variableName,
-            String encodedValue, boolean immediate, char type) {
+            Object value, boolean immediate) {
+        // note that type is now deduced from value
         // TODO could eliminate invocations of same shared variable setter
-        addMethodInvocationToQueue(paintableId, new MethodInvocation(
-                paintableId, UPDATE_VARIABLE_METHOD, new String[] {
-                        variableName, String.valueOf(type), encodedValue }),
+        addMethodInvocationToQueue(new MethodInvocation(paintableId,
+                UPDATE_VARIABLE_METHOD, new Object[] { variableName, value }),
                 immediate);
     }
 
-    private void addMethodInvocationToQueue(String paintableId,
-            MethodInvocation invocation, boolean immediate) {
+    private void addMethodInvocationToQueue(MethodInvocation invocation,
+            boolean immediate) {
         pendingInvocations.add(invocation);
         if (immediate) {
             sendPendingVariableChanges();
@@ -1185,7 +1183,6 @@ public class ApplicationConnection {
 
             JSONArray reqJson = new JSONArray();
 
-            // TODO support typed parameters
             for (MethodInvocation invocation : pendingInvocations) {
                 JSONArray invocationJson = new JSONArray();
                 invocationJson.set(0,
@@ -1194,14 +1191,16 @@ public class ApplicationConnection {
                         new JSONString(invocation.getMethodName()));
                 JSONArray paramJson = new JSONArray();
                 for (int i = 0; i < invocation.getParameters().length; ++i) {
-                    paramJson.set(i, new JSONString(
-                            invocation.getParameters()[i]));
+                    // TODO non-static encoder? type registration?
+                    paramJson.set(i, JsonEncoder.encode(
+                            invocation.getParameters()[i], getPaintableMap()));
                 }
                 invocationJson.set(2, paramJson);
                 reqJson.set(reqJson.size(), invocationJson);
             }
 
-            req.append(reqJson.toString());
+            // escape burst separators (if any)
+            req.append(escapeBurstContents(reqJson.toString()));
 
             pendingInvocations.clear();
             // Append all the bursts to this synchronous request
@@ -1248,8 +1247,7 @@ public class ApplicationConnection {
      */
     public void updateVariable(String paintableId, String variableName,
             VPaintable newValue, boolean immediate) {
-        String pid = paintableMap.getPid(newValue);
-        addVariableToQueue(paintableId, variableName, pid, immediate, 'p');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1272,8 +1270,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             String newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName,
-                escapeVariableValue(newValue), immediate, 's');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1296,8 +1293,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             int newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName, "" + newValue, immediate,
-                'i');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1320,8 +1316,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             long newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName, "" + newValue, immediate,
-                'l');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1344,8 +1339,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             float newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName, "" + newValue, immediate,
-                'f');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1368,8 +1362,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             double newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName, "" + newValue, immediate,
-                'd');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1392,8 +1385,7 @@ public class ApplicationConnection {
 
     public void updateVariable(String paintableId, String variableName,
             boolean newValue, boolean immediate) {
-        addVariableToQueue(paintableId, variableName, newValue ? "true"
-                : "false", immediate, 'b');
+        addVariableToQueue(paintableId, variableName, newValue, immediate);
     }
 
     /**
@@ -1409,55 +1401,13 @@ public class ApplicationConnection {
      * @param variableName
      *            the name of the variable
      * @param map
-     *            the new value to be sent
+     *            the new values to be sent
      * @param immediate
      *            true if the update is to be sent as soon as possible
      */
     public void updateVariable(String paintableId, String variableName,
             Map<String, Object> map, boolean immediate) {
-        final StringBuffer buf = new StringBuffer();
-        Iterator<String> iterator = map.keySet().iterator();
-        while (iterator.hasNext()) {
-            String key = iterator.next();
-            Object value = map.get(key);
-            char transportType = getTransportType(value);
-            buf.append(transportType);
-            buf.append(escapeVariableValue(key));
-            buf.append(VAR_ARRAYITEM_SEPARATOR);
-            if (transportType == 'p') {
-                buf.append(paintableMap.getPid((VPaintable) value));
-            } else {
-                buf.append(escapeVariableValue(String.valueOf(value)));
-            }
-
-            if (iterator.hasNext()) {
-                buf.append(VAR_ARRAYITEM_SEPARATOR);
-            }
-        }
-
-        addVariableToQueue(paintableId, variableName, buf.toString(),
-                immediate, 'm');
-    }
-
-    private char getTransportType(Object value) {
-        if (value instanceof String) {
-            return 's';
-        } else if (value instanceof VPaintableWidget) {
-            return 'p';
-        } else if (value instanceof Boolean) {
-            return 'b';
-        } else if (value instanceof Integer) {
-            return 'i';
-        } else if (value instanceof Float) {
-            return 'f';
-        } else if (value instanceof Double) {
-            return 'd';
-        } else if (value instanceof Long) {
-            return 'l';
-        } else if (value instanceof Enum) {
-            return 's'; // transported as string representation
-        }
-        return 'u';
+        addVariableToQueue(paintableId, variableName, map, immediate);
     }
 
     /**
@@ -1473,25 +1423,14 @@ public class ApplicationConnection {
      *            the id of the paintable that owns the variable
      * @param variableName
      *            the name of the variable
-     * @param newValue
+     * @param values
      *            the new value to be sent
      * @param immediate
      *            true if the update is to be sent as soon as possible
      */
     public void updateVariable(String paintableId, String variableName,
             String[] values, boolean immediate) {
-        final StringBuffer buf = new StringBuffer();
-        if (values != null) {
-            for (int i = 0; i < values.length; i++) {
-                buf.append(escapeVariableValue(values[i]));
-                // there will be an extra separator at the end to differentiate
-                // between an empty array and one containing an empty string
-                // only
-                buf.append(VAR_ARRAYITEM_SEPARATOR);
-            }
-        }
-        addVariableToQueue(paintableId, variableName, buf.toString(),
-                immediate, 'c');
+        addVariableToQueue(paintableId, variableName, values, immediate);
     }
 
     /**
@@ -1508,43 +1447,25 @@ public class ApplicationConnection {
      *            the id of the paintable that owns the variable
      * @param variableName
      *            the name of the variable
-     * @param newValue
+     * @param values
      *            the new value to be sent
      * @param immediate
      *            true if the update is to be sent as soon as possible
      */
     public void updateVariable(String paintableId, String variableName,
             Object[] values, boolean immediate) {
-        final StringBuffer buf = new StringBuffer();
-        if (values != null) {
-            for (int i = 0; i < values.length; i++) {
-                if (i > 0) {
-                    buf.append(VAR_ARRAYITEM_SEPARATOR);
-                }
-                Object value = values[i];
-                char transportType = getTransportType(value);
-                // first char tells the type in array
-                buf.append(transportType);
-                if (transportType == 'p') {
-                    buf.append(paintableMap.getPid((VPaintable) value));
-                } else {
-                    buf.append(escapeVariableValue(String.valueOf(value)));
-                }
-            }
-        }
-        addVariableToQueue(paintableId, variableName, buf.toString(),
-                immediate, 'a');
+        addVariableToQueue(paintableId, variableName, values, immediate);
     }
 
     /**
-     * Encode burst and other separator characters in a String for transport
-     * over the network. This protects from separator injection attacks.
+     * Encode burst separator characters in a String for transport over the
+     * network. This protects from separator injection attacks.
      * 
      * @param value
      *            to encode
      * @return encoded value
      */
-    protected String escapeVariableValue(String value) {
+    protected String escapeBurstContents(String value) {
         final StringBuilder result = new StringBuilder();
         for (int i = 0; i < value.length(); ++i) {
             char character = value.charAt(i);
@@ -1552,7 +1473,6 @@ public class ApplicationConnection {
             case VAR_ESCAPE_CHARACTER:
                 // fall-through - escape character is duplicated
             case VAR_BURST_SEPARATOR:
-            case VAR_ARRAYITEM_SEPARATOR:
                 result.append(VAR_ESCAPE_CHARACTER);
                 // encode as letters for easier reading
                 result.append(((char) (character + 0x30)));
index b0142a7b7bc11cd0259de625760f34334115f3a4..bcbe0de22c31f07e69275ae269cf3f5f60bd4bef 100644 (file)
@@ -945,14 +945,18 @@ public class Util {
         if (paintable != null) {
             VConsole.log("\t" + id + " (" + paintable.getClass() + ") :");
             for (MethodInvocation invocation : invocations) {
-                String[] parameters = invocation.getParameters();
+                Object[] parameters = invocation.getParameters();
                 String formattedParams = null;
                 if (ApplicationConnection.UPDATE_VARIABLE_METHOD
                         .equals(invocation.getMethodName())
-                        && parameters.length == 3) {
-                    // name, type, value
-                    formattedParams = parameters[0] + " (" + parameters[1]
-                            + ")" + " : " + parameters[2];
+                        && parameters.length == 2) {
+                    // name, value
+                    Object value = parameters[1];
+                    // TODO paintables inside lists/maps get rendered as
+                    // components in the debug console
+                    String formattedValue = value instanceof Paintable ? c
+                            .getPid((Paintable) value) : String.valueOf(value);
+                    formattedParams = parameters[0] + " : " + formattedValue;
                 }
                 if (null == formattedParams) {
                     formattedParams = (null != parameters) ? parameters
diff --git a/src/com/vaadin/terminal/gwt/client/communication/JsonEncoder.java b/src/com/vaadin/terminal/gwt/client/communication/JsonEncoder.java
new file mode 100644 (file)
index 0000000..a4aaee1
--- /dev/null
@@ -0,0 +1,136 @@
+package com.vaadin.terminal.gwt.client.communication;
+
+import java.util.Map;
+
+import com.google.gwt.json.client.JSONArray;
+import com.google.gwt.json.client.JSONBoolean;
+import com.google.gwt.json.client.JSONNull;
+import com.google.gwt.json.client.JSONObject;
+import com.google.gwt.json.client.JSONString;
+import com.google.gwt.json.client.JSONValue;
+import com.vaadin.terminal.gwt.client.Paintable;
+
+/**
+ * Encoder for converting RPC parameters and other values to JSON for transfer
+ * between the client and the server.
+ * 
+ * Currently, basic data types as well as Map, String[] and Object[] are
+ * supported, where maps and Object[] can contain other supported data types.
+ * 
+ * TODO support bi-directional codec functionality
+ * 
+ * TODO extensible type support
+ * 
+ * @since 7.0
+ */
+public class JsonEncoder {
+
+    public static final char VTYPE_PAINTABLE = 'p';
+    public static final char VTYPE_BOOLEAN = 'b';
+    public static final char VTYPE_DOUBLE = 'd';
+    public static final char VTYPE_FLOAT = 'f';
+    public static final char VTYPE_LONG = 'l';
+    public static final char VTYPE_INTEGER = 'i';
+    public static final char VTYPE_STRING = 's';
+    public static final char VTYPE_ARRAY = 'a';
+    public static final char VTYPE_STRINGARRAY = 'c';
+    public static final char VTYPE_MAP = 'm';
+
+    // TODO is this needed?
+    public static final char VTYPE_UNDEFINED = 'u';
+
+    /**
+     * Encode a value to a JSON representation for transport from the client to
+     * the server.
+     * 
+     * @param value
+     *            value to convert
+     * @param paintableMapper
+     *            mapper from paintables to paintable IDs
+     * @return JSON representation of the value
+     */
+    public static JSONValue encode(Object value,
+            PaintableIdMapper paintableMapper) {
+        char type = getTransportType(value);
+        Object encodedValue = null;
+
+        if (null == value) {
+            // TODO as undefined type?
+            return combineTypeAndValue(VTYPE_UNDEFINED, JSONNull.getInstance());
+        } else if (value instanceof String[]) {
+            String[] array = (String[]) value;
+            JSONArray jsonArray = new JSONArray();
+            for (int i = 0; i < array.length; ++i) {
+                jsonArray.set(i, new JSONString(array[i]));
+            }
+            return combineTypeAndValue(VTYPE_STRINGARRAY, jsonArray);
+        } else if (value instanceof String) {
+            return combineTypeAndValue(VTYPE_STRING, new JSONString(
+                    (String) value));
+        } else if (value instanceof Boolean) {
+            return combineTypeAndValue(VTYPE_BOOLEAN,
+                    JSONBoolean.getInstance((Boolean) value));
+        } else if (value instanceof Object[]) {
+            Object[] array = (Object[]) value;
+            JSONArray jsonArray = new JSONArray();
+            for (int i = 0; i < array.length; ++i) {
+                // TODO handle object graph loops?
+                jsonArray.set(i, encode(array[i], paintableMapper));
+            }
+            return combineTypeAndValue(VTYPE_ARRAY, jsonArray);
+        } else if (value instanceof Map) {
+            Map<String, Object> map = (Map<String, Object>) value;
+            // TODO implement; types for each element
+            JSONObject jsonMap = new JSONObject();
+            for (String mapKey : map.keySet()) {
+                // TODO handle object graph loops?
+                Object mapValue = map.get(mapKey);
+                jsonMap.put(mapKey, encode(mapValue, paintableMapper));
+            }
+            return combineTypeAndValue(VTYPE_MAP, jsonMap);
+        } else if (value instanceof Paintable) {
+            Paintable paintable = (Paintable) value;
+            return combineTypeAndValue(VTYPE_PAINTABLE, new JSONString(
+                    paintableMapper.getPid(paintable)));
+        } else {
+            return combineTypeAndValue(getTransportType(value), new JSONString(
+                    String.valueOf(value)));
+        }
+    }
+
+    private static JSONValue combineTypeAndValue(char type, JSONValue value) {
+        JSONArray outerArray = new JSONArray();
+        outerArray.set(0, new JSONString(String.valueOf(type)));
+        outerArray.set(1, value);
+        return outerArray;
+    }
+
+    private static char getTransportType(Object value) {
+        if (value instanceof String) {
+            return VTYPE_STRING;
+        } else if (value instanceof Paintable) {
+            return VTYPE_PAINTABLE;
+        } else if (value instanceof Boolean) {
+            return VTYPE_BOOLEAN;
+        } else if (value instanceof Integer) {
+            return VTYPE_INTEGER;
+        } else if (value instanceof Float) {
+            return VTYPE_FLOAT;
+        } else if (value instanceof Double) {
+            return VTYPE_DOUBLE;
+        } else if (value instanceof Long) {
+            return VTYPE_LONG;
+        } else if (value instanceof Enum) {
+            return VTYPE_STRING; // transported as string representation
+        } else if (value instanceof String[]) {
+            return VTYPE_STRINGARRAY;
+        } else if (value instanceof Object[]) {
+            return VTYPE_ARRAY;
+        } else if (value instanceof Map) {
+            return VTYPE_MAP;
+        }
+        // TODO throw exception?
+        return VTYPE_UNDEFINED;
+    }
+
+}
index 92ae5283671c8fb77b97e936e4e264ac73f82b75..5fe1846b345ff6f26fc042a44e5d6302ceeee4f7 100644 (file)
@@ -10,11 +10,10 @@ public class MethodInvocation {
 
     private final String paintableId;
     private final String methodName;
-    private final String[] parameters;
+    private final Object[] parameters;
 
-    // TODO Parameters should be an Object[]?
     public MethodInvocation(String paintableId, String methodName,
-            String[] parameters) {
+            Object[] parameters) {
         this.paintableId = paintableId;
         this.methodName = methodName;
         this.parameters = parameters;
@@ -28,7 +27,7 @@ public class MethodInvocation {
         return methodName;
     }
 
-    public String[] getParameters() {
+    public Object[] getParameters() {
         return parameters;
     }
 }
\ No newline at end of file
index 08e8862e853faf39a8828d2300a9eceba564e301..9bc6813f1241853c354bbdf7eeceabf99b08776e 100644 (file)
@@ -37,7 +37,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
-import java.util.StringTokenizer;
 import java.util.UUID;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -89,7 +88,7 @@ import com.vaadin.ui.Root;
  */
 @SuppressWarnings("serial")
 public abstract class AbstractCommunicationManager implements
-        Paintable.RepaintRequestListener, Serializable {
+        Paintable.RepaintRequestListener, PaintableIdMapper, Serializable {
 
     private static final String DASHDASH = "--";
 
@@ -127,26 +126,9 @@ public abstract class AbstractCommunicationManager implements
     /* Variable records indexes */
     private static final int VAR_PID = 0;
     private static final int VAR_METHOD = 1;
-    private static final int VAR_VARNAME = 2;
-    private static final int VAR_TYPE = 3;
-    private static final int VAR_VALUE = 4;
-
-    private static final char VTYPE_PAINTABLE = 'p';
-    private static final char VTYPE_BOOLEAN = 'b';
-    private static final char VTYPE_DOUBLE = 'd';
-    private static final char VTYPE_FLOAT = 'f';
-    private static final char VTYPE_LONG = 'l';
-    private static final char VTYPE_INTEGER = 'i';
-    private static final char VTYPE_STRING = 's';
-    private static final char VTYPE_ARRAY = 'a';
-    private static final char VTYPE_STRINGARRAY = 'c';
-    private static final char VTYPE_MAP = 'm';
 
     public static final char VAR_BURST_SEPARATOR = '\u001d';
 
-    @Deprecated
-    public static final char VAR_ARRAYITEM_SEPARATOR = '\u001c';
-
     public static final char VAR_ESCAPE_CHARACTER = '\u001b';
 
     private final HashMap<Integer, OpenWindowCache> currentlyOpenWindowsInClient = new HashMap<Integer, OpenWindowCache>();
@@ -1150,8 +1132,9 @@ public abstract class AbstractCommunicationManager implements
             }
 
             for (int bi = 1; bi < bursts.length; bi++) {
-                final String burst = bursts[bi];
-                success = handleBurst(request, application2, success, burst);
+                // unescape any encoded separator characters in the burst
+                final String burst = unescapeBurst(bursts[bi]);
+                success &= handleBurst(request, application2, burst);
 
                 // In case that there were multiple bursts, we know that this is
                 // a special synchronous case for closing window. Thus we are
@@ -1184,40 +1167,73 @@ public abstract class AbstractCommunicationManager implements
     /**
      * Helper class for parsing variable change RPC calls.
      * 
-     * TODO refactor the code related to this, maybe eliminate this class
+     * Note that variable changes still only support the old data types and
+     * partly use Vaadin 6 way of encoding of values. Other RPC method calls
+     * support more data types.
+     * 
+     * @since 7.0
      */
-    private static class VariableChange {
+    private class VariableChange {
         private final String name;
-        private final char type;
-        private final String value;
+        private final Object value;
 
-        public VariableChange(MethodInvocation invocation) {
-            name = invocation.getParameters()[0];
-            type = invocation.getParameters()[1].charAt(0);
-            value = invocation.getParameters()[2];
+        public VariableChange(MethodInvocation invocation) throws JSONException {
+            name = (String) invocation.getParameters()[0];
+            value = invocation.getParameters()[1];
         }
 
+        /**
+         * Returns the variable name for the modification.
+         * 
+         * @return variable name
+         */
         public String getName() {
             return name;
         }
 
-        public char getType() {
-            return type;
-        }
-
-        public String getValue() {
+        /**
+         * Returns the (parsed and converted) value of the updated variable.
+         * 
+         * @return variable value
+         */
+        public Object getValue() {
             return value;
         }
     }
 
-    public boolean handleBurst(Object source, Application app, boolean success,
+    /**
+     * Processes a message burst received from the client.
+     * 
+     * A burst can contain any number of RPC calls, including legacy variable
+     * change calls that are processed separately.
+     * 
+     * Consecutive changes to the value of the same variable are combined and
+     * changeVariables() is only called once for them. This preserves the Vaadin
+     * 6 semantics for components and add-ons that do not use Vaadin 7 RPC
+     * directly.
+     * 
+     * @param source
+     * @param app
+     *            application receiving the burst
+     * @param burst
+     *            the content of the burst as a String to be parsed
+     * @return true if the processing of the burst was successful and there were
+     *         no messages to non-existent components
+     */
+    public boolean handleBurst(Object source, Application app,
             final String burst) {
+        boolean success = true;
         try {
             List<MethodInvocation> invocations = parseInvocations(burst);
 
-            // perform the method invocations, grouping consecutive variable
-            // changes for the same paintable
-            // TODO simplify to do each call separately? breaks old semantics
+            // Perform the method invocations, grouping consecutive variable
+            // changes for the same Paintable.
+
+            // Combining of variable changes is currently needed to preserve the
+            // old semantics for any component that relies on them. If the
+            // support for legacy variable change events is removed, each call
+            // can be performed separately and thelogic here simplified.
+
             for (int i = 0; i < invocations.size(); i++) {
                 MethodInvocation invocation = invocations.get(i);
 
@@ -1243,20 +1259,16 @@ public abstract class AbstractCommunicationManager implements
                     // change for a paintable
 
                     Map<String, Object> m = new HashMap<String, Object>();
-                    m.put(change.getName(),
-                            convertVariableValue(change.getType(),
-                                    change.getValue()));
+                    m.put(change.getName(), change.getValue());
                     while (nextInvocation != null
                             && invocation.getPaintableId().equals(
                                     nextInvocation.getPaintableId())
                             && ApplicationConnection.UPDATE_VARIABLE_METHOD
                                     .equals(nextInvocation.getMethodName())) {
-                        change = new VariableChange(invocation);
-                        m.put(change.getName(),
-                                convertVariableValue(change.getType(),
-                                        change.getValue()));
                         i++;
                         invocation = nextInvocation;
+                        change = new VariableChange(invocation);
+                        m.put(change.getName(), change.getValue());
                         if (i + 1 < invocations.size()) {
                             nextInvocation = invocations.get(i + 1);
                         } else {
@@ -1285,7 +1297,7 @@ public abstract class AbstractCommunicationManager implements
                     // after the window has been removed from the
                     // application or the application has closed
                     if ("close".equals(change.getName())
-                            && "true".equals(change.getValue())) {
+                            && Boolean.TRUE.equals(change.getValue())) {
                         // Silently ignore this
                         continue;
                     }
@@ -1301,6 +1313,7 @@ public abstract class AbstractCommunicationManager implements
                     } else {
                         msg += "non-existent component, VAR_PID="
                                 + invocation.getPaintableId();
+                        // TODO should this cause the message to be ignored?
                         success = false;
                     }
                     logger.warning(msg);
@@ -1311,6 +1324,7 @@ public abstract class AbstractCommunicationManager implements
         } catch (JSONException e) {
             logger.warning("Unable to parse RPC call from the client: "
                     + e.getMessage());
+            // TODO or return success = false?
             throw new RuntimeException(e);
         }
 
@@ -1338,10 +1352,10 @@ public abstract class AbstractCommunicationManager implements
             String paintableId = invocationJson.getString(0);
             String methodName = invocationJson.getString(1);
             JSONArray parametersJson = invocationJson.getJSONArray(2);
-            String[] parameters = new String[parametersJson.length()];
-            // TODO support typed parameters
+            Object[] parameters = new Object[parametersJson.length()];
             for (int j = 0; j < parametersJson.length(); ++j) {
-                parameters[j] = parametersJson.getString(j);
+                parameters[j] = JsonDecoder.convertVariableValue(
+                        parametersJson.getJSONArray(j), this);
             }
             MethodInvocation invocation = new MethodInvocation(paintableId,
                     methodName, parameters);
@@ -1462,111 +1476,15 @@ public abstract class AbstractCommunicationManager implements
 
     }
 
-    private Object convertVariableValue(char variableType, String strValue) {
-        Object val = null;
-        switch (variableType) {
-        case VTYPE_ARRAY:
-            val = convertArray(strValue);
-            break;
-        case VTYPE_MAP:
-            val = convertMap(strValue);
-            break;
-        case VTYPE_STRINGARRAY:
-            val = convertStringArray(strValue);
-            break;
-        case VTYPE_STRING:
-            // decode encoded separators
-            val = decodeVariableValue(strValue);
-            break;
-        case VTYPE_INTEGER:
-            val = Integer.valueOf(strValue);
-            break;
-        case VTYPE_LONG:
-            val = Long.valueOf(strValue);
-            break;
-        case VTYPE_FLOAT:
-            val = Float.valueOf(strValue);
-            break;
-        case VTYPE_DOUBLE:
-            val = Double.valueOf(strValue);
-            break;
-        case VTYPE_BOOLEAN:
-            val = Boolean.valueOf(strValue);
-            break;
-        case VTYPE_PAINTABLE:
-            val = idPaintableMap.get(strValue);
-            break;
-        }
-
-        return val;
-    }
-
-    private Object convertMap(String strValue) {
-        String[] parts = strValue
-                .split(String.valueOf(VAR_ARRAYITEM_SEPARATOR));
-        HashMap<String, Object> map = new HashMap<String, Object>();
-        for (int i = 0; i < parts.length; i += 2) {
-            String key = parts[i];
-            if (key.length() > 0) {
-                char variabletype = key.charAt(0);
-                // decode encoded separators
-                String decodedValue = decodeVariableValue(parts[i + 1]);
-                String decodedKey = decodeVariableValue(key.substring(1));
-                Object value = convertVariableValue(variabletype, decodedValue);
-                map.put(decodedKey, value);
-            }
-        }
-        return map;
-    }
-
-    private String[] convertStringArray(String strValue) {
-        // need to return delimiters and filter them out; otherwise empty
-        // strings are lost
-        // an extra empty delimiter at the end is automatically eliminated
-        final String arrayItemSeparator = String
-                .valueOf(VAR_ARRAYITEM_SEPARATOR);
-        StringTokenizer tokenizer = new StringTokenizer(strValue,
-                arrayItemSeparator, true);
-        List<String> tokens = new ArrayList<String>();
-        String prevToken = arrayItemSeparator;
-        while (tokenizer.hasMoreTokens()) {
-            String token = tokenizer.nextToken();
-            if (!arrayItemSeparator.equals(token)) {
-                // decode encoded separators
-                tokens.add(decodeVariableValue(token));
-            } else if (arrayItemSeparator.equals(prevToken)) {
-                tokens.add("");
-            }
-            prevToken = token;
-        }
-        return tokens.toArray(new String[tokens.size()]);
-    }
-
-    private Object convertArray(String strValue) {
-        String[] val = strValue.split(String.valueOf(VAR_ARRAYITEM_SEPARATOR));
-        if (val.length == 0 || (val.length == 1 && val[0].length() == 0)) {
-            return new Object[0];
-        }
-        Object[] values = new Object[val.length];
-        for (int i = 0; i < values.length; i++) {
-            String string = val[i];
-            // first char of string is type
-            char variableType = string.charAt(0);
-            values[i] = convertVariableValue(variableType, string.substring(1));
-        }
-        return values;
-    }
-
     /**
-     * Decode encoded burst and other separator characters in a variable value
-     * String received from the client. This protects from separator injection
-     * attacks.
+     * Unescape encoded burst separator characters in a burst received from the
+     * client. This protects from separator injection attacks.
      * 
      * @param encodedValue
      *            to decode
      * @return decoded value
      */
-    protected String decodeVariableValue(String encodedValue) {
+    protected String unescapeBurst(String encodedValue) {
         final StringBuilder result = new StringBuilder();
         final StringCharacterIterator iterator = new StringCharacterIterator(
                 encodedValue);
@@ -1580,7 +1498,6 @@ public abstract class AbstractCommunicationManager implements
                     result.append(VAR_ESCAPE_CHARACTER);
                     break;
                 case VAR_BURST_SEPARATOR + 0x30:
-                case VAR_ARRAYITEM_SEPARATOR + 0x30:
                     // +0x30 makes these letters for easier reading
                     result.append((char) (character - 0x30));
                     break;
@@ -1796,6 +1713,10 @@ public abstract class AbstractCommunicationManager implements
         outWriter.print("for(;;);[{");
     }
 
+    public Paintable getPaintable(String paintableId) {
+        return idPaintableMap.get(paintableId);
+    }
+
     /**
      * Gets the Paintable Id. If Paintable has debug id set it will be used
      * prefixed with "PID_S". Otherwise a sequenced ID is created.
diff --git a/src/com/vaadin/terminal/gwt/server/JsonDecoder.java b/src/com/vaadin/terminal/gwt/server/JsonDecoder.java
new file mode 100644 (file)
index 0000000..c7aa41f
--- /dev/null
@@ -0,0 +1,120 @@
+package com.vaadin.terminal.gwt.server;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+
+import com.vaadin.external.json.JSONArray;
+import com.vaadin.external.json.JSONException;
+import com.vaadin.external.json.JSONObject;
+import com.vaadin.terminal.Paintable;
+import com.vaadin.terminal.gwt.client.communication.JsonEncoder;
+
+/**
+ * Decoder for converting RPC parameters and other values from JSON in transfer
+ * between the client and the server.
+ * 
+ * TODO support bi-directional codec functionality
+ * 
+ * @since 7.0
+ */
+public class JsonDecoder {
+
+    /**
+     * Convert a JSON array with two elements (type and value) into a
+     * server-side type, recursively if necessary.
+     * 
+     * @param value
+     *            JSON array with two elements
+     * @param idMapper
+     *            mapper from paintable ID to {@link Paintable} objects
+     * @return converted value (does not contain JSON types)
+     * @throws JSONException
+     *             if the conversion fails
+     */
+    public static Object convertVariableValue(JSONArray value,
+            PaintableIdMapper idMapper) throws JSONException {
+        return convertVariableValue(value.getString(0).charAt(0), value.get(1),
+                idMapper);
+    }
+
+    private static Object convertVariableValue(char variableType, Object value,
+            PaintableIdMapper idMapper) throws JSONException {
+        Object val = null;
+        // TODO type checks etc.
+        switch (variableType) {
+        case JsonEncoder.VTYPE_ARRAY:
+            val = convertArray((JSONArray) value, idMapper);
+            break;
+        case JsonEncoder.VTYPE_MAP:
+            val = convertMap((JSONObject) value, idMapper);
+            break;
+        case JsonEncoder.VTYPE_STRINGARRAY:
+            val = convertStringArray((JSONArray) value);
+            break;
+        case JsonEncoder.VTYPE_STRING:
+            val = value;
+            break;
+        case JsonEncoder.VTYPE_INTEGER:
+            // TODO handle properly
+            val = Integer.valueOf(String.valueOf(value));
+            break;
+        case JsonEncoder.VTYPE_LONG:
+            // TODO handle properly
+            val = Long.valueOf(String.valueOf(value));
+            break;
+        case JsonEncoder.VTYPE_FLOAT:
+            // TODO handle properly
+            val = Float.valueOf(String.valueOf(value));
+            break;
+        case JsonEncoder.VTYPE_DOUBLE:
+            // TODO handle properly
+            val = Double.valueOf(String.valueOf(value));
+            break;
+        case JsonEncoder.VTYPE_BOOLEAN:
+            // TODO handle properly
+            val = Boolean.valueOf(String.valueOf(value));
+            break;
+        case JsonEncoder.VTYPE_PAINTABLE:
+            // TODO handle properly
+            val = idMapper.getPaintable(String.valueOf(value));
+            break;
+        }
+
+        return val;
+    }
+
+    private static Object convertMap(JSONObject jsonMap,
+            PaintableIdMapper idMapper) throws JSONException {
+        HashMap<String, Object> map = new HashMap<String, Object>();
+        Iterator<String> it = jsonMap.keys();
+        while (it.hasNext()) {
+            String key = it.next();
+            map.put(key,
+                    convertVariableValue(jsonMap.getJSONArray(key), idMapper));
+        }
+        return map;
+    }
+
+    private static String[] convertStringArray(JSONArray jsonArray)
+            throws JSONException {
+        List<String> tokens = new ArrayList<String>();
+        for (int i = 0; i < jsonArray.length(); ++i) {
+            tokens.add(jsonArray.getString(i));
+        }
+        return tokens.toArray(new String[tokens.size()]);
+    }
+
+    private static Object convertArray(JSONArray jsonArray,
+            PaintableIdMapper idMapper) throws JSONException {
+        List<Object> tokens = new ArrayList<Object>();
+        for (int i = 0; i < jsonArray.length(); ++i) {
+            // each entry always has two elements: type and value
+            JSONArray entryArray = jsonArray.getJSONArray(i);
+            tokens.add(convertVariableValue(entryArray, idMapper));
+        }
+        return tokens.toArray(new Object[tokens.size()]);
+    }
+
+}
diff --git a/src/com/vaadin/terminal/gwt/server/PaintableIdMapper.java b/src/com/vaadin/terminal/gwt/server/PaintableIdMapper.java
new file mode 100644 (file)
index 0000000..d480eac
--- /dev/null
@@ -0,0 +1,20 @@
+package com.vaadin.terminal.gwt.server;
+
+import com.vaadin.terminal.Paintable;
+
+/**
+ * Mapper between server side paintable IDs and the actual {@link Paintable}
+ * objects.
+ * 
+ * @since 7.0
+ */
+public interface PaintableIdMapper {
+    /**
+     * Get the {@link Paintable} instance corresponding to a paintable id.
+     * 
+     * @param paintableId
+     *            id to get
+     * @return {@link Paintable} instance or null if none found
+     */
+    public Paintable getPaintable(String paintableId);
+}