Incorrect multicall implementation

How about some tests? #639608: Provide assertXmlrpc() function to inspect and verify XML-RPC messages might be a requirement, or at least, could help make testing a lot easier.

+++ includes/xmlrpc.inc	16 Dec 2009 15:44:09 -0000
@@ -475,8 +475,22 @@ function _xmlrpc() {
+  // Message must be OK, but each individual call needn't be

This is somewhat fuzzy. Care to provide a better comment? Looks good, and it pretty easy to understand, but let's come up with a nicely written comment. Should be RTBC.

+++ includes/xmlrpc.inc	16 Dec 2009 22:10:19 -0000
@@ -475,8 +475,29 @@ function _xmlrpc() {
+  // To arrive here, the message must be OK

What means "OK" ?

+++ includes/xmlrpc.inc	16 Dec 2009 22:10:19 -0000
@@ -475,8 +475,29 @@ function _xmlrpc() {
+  /**
+   * However, since this is a multicall, each individual method call
+   * performed on the server side may have caused an error, while not
+   * preventing subsequent methods and the overall multicall from
+   * succeeding. Therefore we store the individual error for each
+   * method within the multicall result.
+   */

Wrong comment style. We do not use multi-line comments within function bodies; always use inline comments here.

In addition, this comment reads a bit lengthy - just clarifying what the expected result for multicalls (and difference to single methods) is should be sufficient.

+++ includes/xmlrpc.inc	16 Dec 2009 22:10:19 -0000
@@ -475,8 +475,29 @@ function _xmlrpc() {
+      if (array_keys($result) == array(0)) {
+        $ret[] = $result[0];
+      }
+      else {
+        $ret[] = xmlrpc_error($result['faultCode'], $result['faultString']);
+      }

What is happening here?

Also, we should not abbreviate $ret[urn]

This looks valid to me. Let's see if there are any final reviewers.

Looks indeed much better now.

+++ includes/xmlrpc.inc	19 Dec 2009 08:52:15 -0000
@@ -430,13 +430,16 @@ function xmlrpc_base64_get_xml($xmlrpc_b
+     Either the return value of the method on success, or FALSE.
+ *   If FALSE is returned, see xmlrpc_errno() and xmlrpc_error_msg().

Should not wrap early, but at 80 chars instead.

+++ includes/xmlrpc.inc	19 Dec 2009 08:52:15 -0000
@@ -430,13 +430,16 @@ function xmlrpc_base64_get_xml($xmlrpc_b
+     Either the return value of the method on success, or FALSE.

Missing leading ' * '.

+++ includes/xmlrpc.inc	19 Dec 2009 08:52:15 -0000
@@ -430,13 +430,16 @@ function xmlrpc_base64_get_xml($xmlrpc_b
+ *   For a non-multicall request:
+ *     The result just as if this had been a local function call.
+ *   For a multicall request:
+ *     An array of results. Each result will either be a one-element array
+ *     containing the result returned by the method called, or an xmlrpc_error
+ *     object if the call failed.

Use a Doxygen list here instead? See http://drupal.org/node/1354 for proper formatting of lists.

This review is powered by Dreditor.

My bad.

This can be backported to D6.

Committed to CVS HEAD. Changing version and status.

Reviewed the D6 version. Looks great.

Subscribing.