]> source.dussan.org Git - vaadin-framework.git/commitdiff
Use simple JSON for RPC calls from client to server (#8279).
authorHenri Sara <hesara@vaadin.com>
Wed, 18 Jan 2012 17:26:47 +0000 (19:26 +0200)
committerHenri Sara <hesara@vaadin.com>
Wed, 25 Jan 2012 11:31:46 +0000 (13:31 +0200)
A MethodInvocation can have multiple parameters, but only String is
supported as a data type for them. Values to updateVariable() are still
encoded using old mechanisms and sent as name/type/value string tuples.

src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml
src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
src/com/vaadin/terminal/gwt/client/Util.java
src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java
src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java

index 9bc05dee2eaabe1e19b6631af53966a622ee35ba..92b23836e3aed74fe7458dd36a552da8996199ae 100644 (file)
@@ -9,6 +9,8 @@
        <inherits name="com.google.gwt.user.User" />
 
        <inherits name="com.google.gwt.http.HTTP" />
+       
+       <inherits name="com.google.gwt.json.JSON" />
 
        <source path="client" />
 
index a7f146be16fc9879a2108ff3be09938caaf03394..6f7d6818f9b3aa758849d63744b1c6505b6807a9 100644 (file)
@@ -23,6 +23,8 @@ import com.google.gwt.http.client.RequestBuilder;
 import com.google.gwt.http.client.RequestCallback;
 import com.google.gwt.http.client.RequestException;
 import com.google.gwt.http.client.Response;
+import com.google.gwt.json.client.JSONArray;
+import com.google.gwt.json.client.JSONString;
 import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Element;
@@ -76,12 +78,9 @@ public class ApplicationConnection {
 
     public static final String UPDATE_VARIABLE_METHOD = "v";
 
-    public static final char VAR_RECORD_SEPARATOR = '\u001e';
-
-    public static final char VAR_FIELD_SEPARATOR = '\u001f';
-
     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';
@@ -1109,10 +1108,10 @@ public class ApplicationConnection {
     private void addVariableToQueue(String paintableId, String variableName,
             String encodedValue, boolean immediate, char type) {
         // TODO could eliminate invocations of same shared variable setter
-        String param = variableName + VAR_FIELD_SEPARATOR + type
-                + VAR_FIELD_SEPARATOR + encodedValue;
         addMethodInvocationToQueue(paintableId, new MethodInvocation(
-                paintableId, UPDATE_VARIABLE_METHOD, param), immediate);
+                paintableId, UPDATE_VARIABLE_METHOD, new String[] {
+                        variableName, String.valueOf(type), encodedValue }),
+                immediate);
     }
 
     private void addMethodInvocationToQueue(String paintableId,
@@ -1183,20 +1182,27 @@ public class ApplicationConnection {
             if (ApplicationConfiguration.isDebugMode()) {
                 Util.logVariableBurst(this, pendingInvocations);
             }
-            // TODO use JSON for messages
-            for (int i = 0; i < pendingInvocations.size(); i++) {
-                if (i > 0) {
-                    req.append(VAR_RECORD_SEPARATOR);
+
+            JSONArray reqJson = new JSONArray();
+
+            // TODO support typed parameters
+            for (MethodInvocation invocation : pendingInvocations) {
+                JSONArray invocationJson = new JSONArray();
+                invocationJson.set(0,
+                        new JSONString(invocation.getPaintableId()));
+                invocationJson.set(1,
+                        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]));
                 }
-                MethodInvocation invocation = pendingInvocations.get(i);
-                req.append(invocation.getPaintableId());
-                req.append(VAR_FIELD_SEPARATOR);
-                req.append(invocation.getMethodName());
-                req.append(VAR_FIELD_SEPARATOR);
-                // TODO support multiple parameters
-                req.append(invocation.getParameters());
+                invocationJson.set(2, paramJson);
+                reqJson.set(reqJson.size(), invocationJson);
             }
 
+            req.append(reqJson.toString());
+
             pendingInvocations.clear();
             // Append all the bursts to this synchronous request
             if (forceSync && !pendingBursts.isEmpty()) {
@@ -1531,9 +1537,8 @@ public class ApplicationConnection {
     }
 
     /**
-     * Encode burst, record, field and array item separator characters in a
-     * String for transport over the network. This protects from separator
-     * injection attacks.
+     * Encode burst and other separator characters in a String for transport
+     * over the network. This protects from separator injection attacks.
      * 
      * @param value
      *            to encode
@@ -1547,8 +1552,6 @@ public class ApplicationConnection {
             case VAR_ESCAPE_CHARACTER:
                 // fall-through - escape character is duplicated
             case VAR_BURST_SEPARATOR:
-            case VAR_RECORD_SEPARATOR:
-            case VAR_FIELD_SEPARATOR:
             case VAR_ARRAYITEM_SEPARATOR:
                 result.append(VAR_ESCAPE_CHARACTER);
                 // encode as letters for easier reading
index 2789a730781bda628e437fad93300e3e1c39fa69..b0142a7b7bc11cd0259de625760f34334115f3a4 100644 (file)
@@ -945,25 +945,18 @@ public class Util {
         if (paintable != null) {
             VConsole.log("\t" + id + " (" + paintable.getClass() + ") :");
             for (MethodInvocation invocation : invocations) {
-                String formattedParams = invocation.getParameters();
+                String[] parameters = invocation.getParameters();
+                String formattedParams = null;
                 if (ApplicationConnection.UPDATE_VARIABLE_METHOD
-                        .equals(invocation.getMethodName())) {
-                    // if an updateVariable call, format as variable changes
-                    // ensure with limit that also trailing parameters are
-                    // included
-                    String[] split = invocation
-                            .getParameters()
-                            .split(String
-                                    .valueOf(ApplicationConnection.VAR_FIELD_SEPARATOR),
-                                    3);
-
-                    // unless of correct length, do not reformat
-                    if (split.length == 3) {
-                        // TODO needs to be reworked
-                        // name, type, value
-                        formattedParams = split[0] + " (" + split[1] + ")"
-                                + " : " + split[2];
-                    }
+                        .equals(invocation.getMethodName())
+                        && parameters.length == 3) {
+                    // name, type, value
+                    formattedParams = parameters[0] + " (" + parameters[1]
+                            + ")" + " : " + parameters[2];
+                }
+                if (null == formattedParams) {
+                    formattedParams = (null != parameters) ? parameters
+                            .toString() : null;
                 }
                 VConsole.log("\t\t" + invocation.getMethodName() + "("
                         + formattedParams + ")");
index c07427fc9db237fcd70761924cd6e41b456ac7b9..92ae5283671c8fb77b97e936e4e264ac73f82b75 100644 (file)
@@ -10,11 +10,11 @@ public class MethodInvocation {
 
     private final String paintableId;
     private final String methodName;
-    private final String parameters;
+    private final String[] parameters;
 
     // TODO Parameters should be an Object[]?
     public MethodInvocation(String paintableId, String methodName,
-            String parameters) {
+            String[] parameters) {
         this.paintableId = paintableId;
         this.methodName = methodName;
         this.parameters = parameters;
@@ -28,7 +28,7 @@ public class MethodInvocation {
         return methodName;
     }
 
-    public String getParameters() {
+    public String[] getParameters() {
         return parameters;
     }
 }
\ No newline at end of file
index c71bb0ea10fc085af15ae01d044961a2ee9db66f..c359d70ea7c59b303cdec7d0e44d2b9ae08fe297 100644 (file)
@@ -45,6 +45,7 @@ import java.util.logging.Logger;
 import com.vaadin.Application;
 import com.vaadin.Application.SystemMessages;
 import com.vaadin.RootRequiresMoreInformationException;
+import com.vaadin.external.json.JSONArray;
 import com.vaadin.external.json.JSONException;
 import com.vaadin.external.json.JSONObject;
 import com.vaadin.terminal.CombinedRequest;
@@ -62,6 +63,7 @@ import com.vaadin.terminal.VariableOwner;
 import com.vaadin.terminal.WrappedRequest;
 import com.vaadin.terminal.WrappedResponse;
 import com.vaadin.terminal.gwt.client.ApplicationConnection;
+import com.vaadin.terminal.gwt.client.communication.MethodInvocation;
 import com.vaadin.terminal.gwt.server.BootstrapHandler.BootstrapContext;
 import com.vaadin.terminal.gwt.server.ComponentSizeValidator.InvalidLayout;
 import com.vaadin.ui.AbstractComponent;
@@ -140,12 +142,9 @@ public abstract class AbstractCommunicationManager implements
     private static final char VTYPE_STRINGARRAY = 'c';
     private static final char VTYPE_MAP = 'm';
 
-    private static final char VAR_RECORD_SEPARATOR = '\u001e';
-
-    private static final char VAR_FIELD_SEPARATOR = '\u001f';
-
     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';
@@ -1182,104 +1181,173 @@ public abstract class AbstractCommunicationManager implements
         return success;
     }
 
+    /**
+     * Helper class for parsing variable change RPC calls.
+     * 
+     * TODO refactor the code related to this, maybe eliminate this class
+     */
+    private static class VariableChange {
+        private final String name;
+        private final char type;
+        private final String value;
+
+        public VariableChange(MethodInvocation invocation) {
+            name = invocation.getParameters()[0];
+            type = invocation.getParameters()[1].charAt(0);
+            value = invocation.getParameters()[2];
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        public char getType() {
+            return type;
+        }
+
+        public String getValue() {
+            return value;
+        }
+    }
+
     public boolean handleBurst(Object source, Application app, boolean success,
             final String burst) {
-        // extract variables to two dim string array
-        final String[] tmp = burst.split(String.valueOf(VAR_RECORD_SEPARATOR));
-        // TODO support variable number of parameters
-        final String[][] variableRecords = new String[tmp.length][5];
-        for (int i = 0; i < tmp.length; i++) {
-            // ensure with limit that also trailing parameters are included
-            variableRecords[i] = tmp[i].split(
-                    String.valueOf(VAR_FIELD_SEPARATOR), 5);
-        }
-
-        for (int i = 0; i < variableRecords.length; i++) {
-            String[] variable = variableRecords[i];
-            String[] nextVariable = null;
-            if (i + 1 < variableRecords.length) {
-                nextVariable = variableRecords[i + 1];
-            }
-            final VariableOwner owner = getVariableOwner(variable[VAR_PID]);
-            final String methodName = variable[VAR_METHOD];
-            if (!ApplicationConnection.UPDATE_VARIABLE_METHOD
-                    .equals(methodName)) {
-                // TODO handle other RPC calls
-                continue;
-            }
-            if (owner != null && owner.isEnabled()) {
-                Map<String, Object> m;
-                if (nextVariable != null
-                        && variable[VAR_PID].equals(nextVariable[VAR_PID])) {
-                    // we have more than one value changes in row for
-                    // one variable owner, collect them in HashMap
-                    m = new HashMap<String, Object>();
-                    m.put(variable[VAR_VARNAME],
-                            convertVariableValue(variable[VAR_TYPE].charAt(0),
-                                    variable[VAR_VALUE]));
-                } else {
-                    // use optimized single value map
-                    m = Collections.singletonMap(
-                            variable[VAR_VARNAME],
-                            convertVariableValue(variable[VAR_TYPE].charAt(0),
-                                    variable[VAR_VALUE]));
-                }
+        try {
+            List<MethodInvocation> invocations = parseInvocations(burst);
 
-                // collect following variable changes for this owner
-                while (nextVariable != null
-                        && variable[VAR_PID].equals(nextVariable[VAR_PID])) {
-                    i++;
-                    variable = nextVariable;
-                    if (i + 1 < variableRecords.length) {
-                        nextVariable = variableRecords[i + 1];
-                    } else {
-                        nextVariable = null;
-                    }
-                    m.put(variable[VAR_VARNAME],
-                            convertVariableValue(variable[VAR_TYPE].charAt(0),
-                                    variable[VAR_VALUE]));
+            // perform the method invocations, grouping consecutive variable
+            // changes for the same paintable
+            // TODO simplify to do each call separately? breaks old semantics
+            for (int i = 0; i < invocations.size(); i++) {
+                MethodInvocation invocation = invocations.get(i);
+
+                MethodInvocation nextInvocation = null;
+                if (i + 1 < invocations.size()) {
+                    nextInvocation = invocations.get(i + 1);
                 }
-                try {
-                    changeVariables(source, owner, m);
-                } catch (Exception e) {
-                    if (owner instanceof Component) {
-                        handleChangeVariablesError(app, (Component) owner, e, m);
-                    } else {
-                        // TODO DragDropService error handling
-                        throw new RuntimeException(e);
+
+                final VariableOwner owner = getVariableOwner(invocation
+                        .getPaintableId());
+                final String methodName = invocation.getMethodName();
+
+                if (owner != null && owner.isEnabled()) {
+                    if (!ApplicationConnection.UPDATE_VARIABLE_METHOD
+                            .equals(methodName)) {
+                        // TODO handle other RPC calls
+                        continue;
                     }
-                }
-            } else {
 
-                // Handle special case where window-close is called
-                // after the window has been removed from the
-                // application or the application has closed
-                if ("close".equals(variable[VAR_VARNAME])
-                        && "true".equals(variable[VAR_VALUE])) {
-                    // Silently ignore this
-                    continue;
-                }
+                    VariableChange change = new VariableChange(invocation);
+
+                    // TODO could optimize with a single value map if only one
+                    // change for a paintable
+
+                    Map<String, Object> m = new HashMap<String, Object>();
+                    m.put(change.getName(),
+                            convertVariableValue(change.getType(),
+                                    change.getValue()));
+                    while (nextInvocation != null
+                            && invocation.getPaintableId().equals(
+                                    nextInvocation.getPaintableId())) {
+                        change = new VariableChange(invocation);
+                        m.put(change.getName(),
+                                convertVariableValue(change.getType(),
+                                        change.getValue()));
+                        i++;
+                        invocation = nextInvocation;
+                        if (i + 1 < invocations.size()) {
+                            nextInvocation = invocations.get(i + 1);
+                        } else {
+                            nextInvocation = null;
+                        }
+                    }
 
-                // Ignore variable change
-                String msg = "Warning: Ignoring variable change for ";
-                if (owner != null) {
-                    msg += "disabled component " + owner.getClass();
-                    String caption = ((Component) owner).getCaption();
-                    if (caption != null) {
-                        msg += ", caption=" + caption;
+                    try {
+                        changeVariables(source, owner, m);
+                    } catch (Exception e) {
+                        if (owner instanceof Component) {
+                            handleChangeVariablesError(app, (Component) owner,
+                                    e, m);
+                        } else {
+                            // TODO DragDropService error handling
+                            throw new RuntimeException(e);
+                        }
                     }
                 } else {
-                    msg += "non-existent component, VAR_PID="
-                            + variable[VAR_PID];
-                    success = false;
+                    // TODO convert window close to a separate RPC call, not a
+                    // variable change
+
+                    VariableChange change = new VariableChange(invocation);
+
+                    // Handle special case where window-close is called
+                    // after the window has been removed from the
+                    // application or the application has closed
+                    if ("close".equals(change.getName())
+                            && "true".equals(change.getValue())) {
+                        // Silently ignore this
+                        continue;
+                    }
+
+                    // Ignore variable change
+                    String msg = "Warning: Ignoring RPC call for ";
+                    if (owner != null) {
+                        msg += "disabled component " + owner.getClass();
+                        String caption = ((Component) owner).getCaption();
+                        if (caption != null) {
+                            msg += ", caption=" + caption;
+                        }
+                    } else {
+                        msg += "non-existent component, VAR_PID="
+                                + invocation.getPaintableId();
+                        success = false;
+                    }
+                    logger.warning(msg);
+                    continue;
                 }
-                logger.warning(msg);
-                continue;
             }
+
+        } catch (JSONException e) {
+            logger.warning("Unable to parse RPC call from the client: "
+                    + e.getMessage());
+            throw new RuntimeException(e);
         }
+
         return success;
     }
 
+    /**
+     * Parse a message burst from the client into a list of MethodInvocation
+     * instances.
+     * 
+     * @param burst
+     *            message string (JSON)
+     * @return list of MethodInvocation to perform
+     * @throws JSONException
+     */
+    private List<MethodInvocation> parseInvocations(final String burst)
+            throws JSONException {
+        JSONArray invocationsJson = new JSONArray(burst);
+
+        ArrayList<MethodInvocation> invocations = new ArrayList<MethodInvocation>();
+
+        // parse JSON to MethodInvocations
+        for (int i = 0; i < invocationsJson.length(); ++i) {
+            JSONArray invocationJson = invocationsJson.getJSONArray(i);
+            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
+            for (int j = 0; j < parametersJson.length(); ++j) {
+                parameters[j] = parametersJson.getString(j);
+            }
+            MethodInvocation invocation = new MethodInvocation(paintableId,
+                    methodName, parameters);
+            invocations.add(invocation);
+        }
+        return invocations;
+    }
+
     protected void changeVariables(Object source, final VariableOwner owner,
             Map<String, Object> m) {
         owner.changeVariables(source, m);
@@ -1488,9 +1556,9 @@ public abstract class AbstractCommunicationManager implements
     }
 
     /**
-     * Decode encoded burst, record, field and array item separator characters
-     * in a variable value String received from the client. This protects from
-     * separator injection attacks.
+     * Decode encoded burst and other separator characters in a variable value
+     * String received from the client. This protects from separator injection
+     * attacks.
      * 
      * @param encodedValue
      *            to decode
@@ -1510,8 +1578,6 @@ public abstract class AbstractCommunicationManager implements
                     result.append(VAR_ESCAPE_CHARACTER);
                     break;
                 case VAR_BURST_SEPARATOR + 0x30:
-                case VAR_RECORD_SEPARATOR + 0x30:
-                case VAR_FIELD_SEPARATOR + 0x30:
                 case VAR_ARRAYITEM_SEPARATOR + 0x30:
                     // +0x30 makes these letters for easier reading
                     result.append((char) (character - 0x30));