Hi,

I have patched my local Drupal instance with the following patch:

$ git diff sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc
diff --git a/sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc b/sites/all/modules/custom/services/servers/rest_server/inc
index b23f191..a628e89 100644
--- a/sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc
+++ b/sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc
@@ -42,7 +42,7 @@ class RESTServerViewBuiltIn extends RESTServerView {
   }
 
   private function render_json($data, $jsonp = FALSE) {
-    $json = str_replace('\\/', '/', json_encode($data));
+    $json = str_replace('\\/', '/', json_encode($data, JSON_FORCE_OBJECT));
     if ($jsonp && isset($_GET['callback'])) {
       return sprintf('%s(%s);', $_GET['callback'], $json);
     }

Without this patch empty field data would return as an empty json array. i.e.:
"field_foo_select": [],

But the same, non empty field would return as a json object:

    "field_foo_select": {
        "und": {
            "0": {
                "value": "bar"
            }
        }
    }, 

This discrepancy (object vs 0 length array) causes issues with clients that consume services, namely JAXB and Jackson. Should this patch be included upstream?

I would like to hear your thoughts.
Thanks,
-Ian Page Hands

Comments

iphands’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, json_JSON_FORC_OBJECT.patch, failed testing.

marcingy’s picture

Status: Needs work » Closed (won't fix)

This is only supported in php 5.3 not 5.2 so won't fix

iphands’s picture

Status: Closed (won't fix) » Needs review

This is only supported in php 5.3 not 5.2 so won't fix

Great point, and thanks for the response.

What about doing something like:

--- a/sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc
+++ b/sites/all/modules/custom/services/servers/rest_server/includes/rest_server.views.inc
@@ -42,7 +42,7 @@ class RESTServerViewBuiltIn extends RESTServerView {
   }

   private function render_json($data, $jsonp = FALSE) {
-    $json = str_replace('\\/', '/', json_encode($data));
+    $json = preg_replace("/(\".*?\"[\s]*\:[\s]*)\[\s*]/s", "$1{\$2}", $json);
     if ($jsonp && isset($_GET['callback'])) {
       return sprintf('%s(%s);', $_GET['callback'], $json);
     }

That would at least fix the case where empty objects become empty arrays. It might also mean that now empty arrays become empty objects though. As far as I can tell though, all values in the map at least open with an object and not an array (when they are non empty that is). So maybe the solution will work.

Also, is there a way to say something like:

$json = Null;
if (PHP_VERSION >= 5.3) {
    $json = str_replace('\\/', '/', json_encode($data, JSON_FORCE_OBJECT));
} else {
    $json = str_replace('\\/', '/', json_encode($data);
    $json = preg_replace("/(\".*?\"[\s]*\:[\s]*)\[\s*]/s", "$1{\$2}", $json);
}
kylebrowning’s picture

This is an api change that cannot happen until services_historical is added to core.

iphands’s picture

StatusFileSize
new3.8 KB

As discussed in #drupal-services, here is a new patch that adds the option to enable forcing the empty PHP map values to render as JSON objects in the endpoint config UI. The default is to simply leave off the option, which would preserve existing API behavior.

Please have a look and let me know if I chose the right place/method to add the UI element.

Also the new patch uses the 'preg_replace("/(\".*?\"[\s]*\:[\s]*)\[\s*]/s", "$1{\$2}", $json);' code to force change [] to {}. It would be cool if there was a way to detect PHP 5.3 and greater, then use the JSON_FORCE_OBJECT param with json_encode() and only use the preg_replace as a fallback, but I am not sure if there is a good/reliable way to get the PHP version info in the render_json() function.

Let me know if something needs to be changed.
Thanks,
-Ian Page Hands

Status: Needs review » Needs work

The last submitted patch, json_array_as_object.patch, failed testing.

iphands’s picture

Status: Needs work » Needs review
StatusFileSize
new3.59 KB

Fixed the paths on the patch. Applies cleanly with -p1

Status: Needs review » Needs work

The last submitted patch, json_array_as_object.patch, failed testing.

iphands’s picture

Status: Needs work » Needs review

#8: json_array_as_object.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, json_array_as_object.patch, failed testing.

kylebrowning’s picture

Category: support » task
Status: Needs work » Postponed

Waiting for services historical.

kylebrowning’s picture

Version: 7.x-3.1 » 7.x-3.x-dev
Status: Postponed » Needs review
ygerasimov’s picture

Status: Needs review » Closed (won't fix)

Lets not have this option exposed via UI. If we do this we will get a lot of other issues about other options for the json_encode (http://www.php.net/manual/en/json.constants.php). I propose to keep it as it is. It is pretty easy to create your own custom formatter that will run json_encode with any option you would like.