[Feature Request] ProtocolBuffersMessage should implement JsonSerializable interface

[dcelasun]

Is it possible to make ProtocolBuffersMessage implement JsonSerializable?

Basically, it could return a key/value pair of all fields & values in the message, like this:

public function jsonSerialize()
{
    return array(
        "foo" => $this->get("foo"),
        "bar" => $this->get("bar"),
    );
}

If you can do this all children of ProtocolBuffersMessage can now be serialized with json_encode which would help immensely with logging etc.

Do you think this is feasible?

[dcelasun]

@chobie Any thoughts on this?

[chobie]

ah, sorry. I've just missed.

I can add JsonSerializable support above 5.4.26 (previous versions doesn't export JsonSerializable pointer for dynamic loaded extension.)

http://www.php.net/ChangeLog-5.php#5.4.26 https://bugs.php.net/bug.php?id=65753

I'll implement in this weekend.

[dcelasun]

Good to hear, thanks!

Edit: Also, the minimum version for the 5.4 series is 5.4.26, and 5.5.10 for the 5.5 series.

[chobie]

I realized supporting jsonSerialize means to add another serializer. I've just implemented prototype on support-jsonserializable branch but it need detailed specification and testcases.

sorry for delay. I'll fix this in next weekend at the latest.

[dcelasun]

Thanks for looking into this. I just tried the support-jsonserializable branch, but it doesn't seem to work properly (json_encode detects recursion and returns false). I've included a minimal test case from work below:

Proto messages:

message RedisNode {
    optional string host = 1;
    optional int32 port = 2;
    optional string name = 3;
    optional int32 index = 4;
}

enum RedisPoolType {
    USER_DATA = 0;
    COMMUNICATION = 1;
    USER_NODE_ID = 2;
    SCORE_BOARD = 3;
}

message RedisPool {
    optional RedisPoolType poolType = 1;
    repeated RedisNode redisNodes = 2;
}

On the PHP side:

$node = new RedisNode();

$node->setHost("127.0.0.1");
$node->setPort(1337);

$pool = new RedisPool();

$pool->setPoolType(1);
$pool->appendRedisNodes($node);

$serialized = $pool->serializeToString();
$unserialized = RedisPool::parseFromString($serialized);

echo "<pre>";

var_dump($unserialized->jsonSerialize());
var_dump(json_encode($unserialized));
var_dump(json_encode($unserialized->jsonSerialize()));
var_dump(json_last_error(), json_last_error_msg());

... and the output:

array (size=2)
  'pooltype' => string '1' (length=1)
  'redisnodes' => 
    array (size=1)
      0 => 
        array (size=2)
          'host' => string '127.0.0.1' (length=9)
          'port' => string '1337' (length=4)
boolean false
boolean false
int 6
string 'Recursion detected' (length=18)

[chobie]

Thanks, this is very helpful :)

[dcelasun]

It seems the recursion error got fixed with this commit. Thanks!

Edit: I removed the rest of this post because I mistakenly thought there was another bug.

[dcelasun]

One more thing, on CentOS 6 using pecl-json-c, I get the following during make test:

PHP Warning:  PHP Startup: Unable to load dynamic library 'path/to/protocolbuffers.so' - /path/to/protocolbuffers.so: undefined symbol: php_json_serializable_ce in Unknown on line 0

A lot of distributions use that extension instead of the builtin one due to license concerns. Still, it also exports the symbol as you can see in this commit.

I think the problem is your makefile loads protocolbuffers.so before the other extensions, so it can't see the symbol.

[chobie]

A lot of distributions use that extension instead of the builtin one due to license concerns. Still, it also exports the symbol as you can see in this commit so I'm not sure what's wrong. Thoughts?

probably loading order problem. could you try to rename protocolbuffers.ini to zzz_protocolbuffers.ini or something?

i'll setup centos box and look into it when I back home.

[dcelasun]

could you try to rename protocolbuffers.ini to zzz_protocolbuffers.ini or something?

Tried aa_protocolbuffers.ini and zz_protocolbuffers.ini, neither worked.

[chobie]

I've tested with CentOS 6.5 and remi repo. but couldn't reproduce.

[chobie@localhost php-protocolbuffers]$ cat /etc/issue
CentOS release 6.5 (Final)
Kernel \r on an \m

[chobie@localhost php-protocolbuffers]$ php -v
PHP 5.5.11 (cli) (built: Apr  3 2014 07:55:14)
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies

[chobie@localhost php-protocolbuffers]$ ls /etc/php.d/
bz2.ini       ctype.ini  dom.ini   fileinfo.ini  gettext.ini  json.ini  posix.ini            shmop.ini      sockets.ini  sysvmsg.ini  sysvshm.ini  tokenizer.ini  xml_wddx.ini   xmlwriter.ini  zip.ini
calendar.ini  curl.ini   exif.ini  ftp.ini       iconv.ini    phar.ini  protocolbuffers.ini  simplexml.ini  sundown.ini  sysvsem.ini  tidy.ini     xml.ini        xmlreader.ini  xsl.ini

[chobie@localhost php-protocolbuffers]$ php --re json | head
Extension [ <persistent> extension #26 json version 1.3.3 ] {

[chobie@localhost php-protocolbuffers]$ php redis_example.php
<pre>array(2) {
  ["pooltype"]=>
  string(1) "1"
  ["redisnodes"]=>
  array(1) {
    [0]=>
    array(2) {
      ["host"]=>
      string(9) "127.0.0.1"
      ["port"]=>
      string(4) "1337"
    }
  }
}
string(66) "{"pooltype":"1","redisnodes":[{"host":"127.0.0.1","port":"1337"}]}"
string(66) "{"pooltype":"1","redisnodes":[{"host":"127.0.0.1","port":"1337"}]}"
int(0)
string(8) "No error"

could you paste nm command outputs like this?

[chobie@localhost php-protocolbuffers]$ nm -D /usr/lib64/php/modules/json.so | grep php_json_serializable_ce
0000000000210e58 B php_json_serializable_ce

[dcelasun]

I think there is a misunderstanding. The extension works fine, the only thing that fails is the make test command during compilation. Here's the relevant part of the Makefile:

test: all
    @if test ! -z "$(PHP_EXECUTABLE)" && test -x "$(PHP_EXECUTABLE)"; then \
            INI_FILE=`$(PHP_EXECUTABLE) -d 'display_errors=stderr' -r 'echo php_ini_loaded_file();' 2> /dev/null`; \
            if test "$$INI_FILE"; then \
                    $(EGREP) -h -v $(PHP_DEPRECATED_DIRECTIVES_REGEX) "$$INI_FILE" > $(top_builddir)/tmp-php.ini; \
            else \
                    echo > $(top_builddir)/tmp-php.ini; \
            fi; \
            INI_SCANNED_PATH=`$(PHP_EXECUTABLE) -d 'display_errors=stderr' -r '$$a = explode(",\n", trim(php_ini_scanned_files())); echo $$a[0];' 2> /dev/null`; \
            if test "$$INI_SCANNED_PATH"; then \
                    INI_SCANNED_PATH=`$(top_srcdir)/build/shtool path -d $$INI_SCANNED_PATH`; \
                    $(EGREP) -h -v $(PHP_DEPRECATED_DIRECTIVES_REGEX) "$$INI_SCANNED_PATH"/*.ini >> $(top_builddir)/tmp-php.ini; \
            fi; \
            TEST_PHP_EXECUTABLE=$(PHP_EXECUTABLE) \
            TEST_PHP_SRCDIR=$(top_srcdir) \
            CC="$(CC)" \
                    $(PHP_EXECUTABLE) -n -c $(top_builddir)/tmp-php.ini $(PHP_TEST_SETTINGS) $(top_srcdir)/run-tests.php -n -c $(top_builddir)/tmp-php.ini -d extension_dir=$(top_builddir)/modules/ $(PHP_TEST_SHARED_EXTENSIONS) $(TESTS); \
            TEST_RESULT_EXIT_CODE=$$?; \
            rm $(top_builddir)/tmp-php.ini; \
            exit $$TEST_RESULT_EXIT_CODE; \
    else \
            echo "ERROR: Cannot run tests without CLI sapi."; \
    fi

[chobie]

exactly, run-test.php doesn't care about another extension.
hmm, I'll look for some workaround about this issue.

[dcelasun]

Again, this is a very minor issue so don't worry about it.

Thanks for implementing json support by the way, I really appreciate it :)

[chobie]

okay, I'll fix that later.

this implementation is still work in progress. it might contains bug. np, also I need this feature :)

[dcelasun]

Hey @chobie, sadly the support-jsonserializable got broken with the latest commits. The last successful build I have is at commit 5f710518f3e43597660cfa7571f9778cafd33fee.

After this commit, the build fails with:

/tmp/php-protocolbuffers/core.c:5:29: fatal error: json_serializer.h: No such file or directory
 #include "json_serializer.h"
                         ^
compilation terminated.
Makefile:212: recipe for target 'core.lo' failed
make: *** [core.lo] Error 1

Finally, even with the last working commit, 2 unit tests fail:

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
ProtocolBuffersMessage::__construct accepts array parameters [tests/600_constructor.phpt]
Check for JsonSerializable implementations [tests/700_jsonserializable.phpt]
=====================================================================

Any ideas?

[chobie]

oops, sorry. I've missed to push commits.

It's not fully tested but almost works fine. remaining task are check error handling (check segv) and write more test cases. probably I'll merge this branch tomorrow.

[dcelasun]

I've just tested your latest commits. It compiles fine and all unit tests pass! Thanks for the quick response :)