Wrong handling of empty objects in graph leading to wrongfully altered JSON

[narcoticfresh]

Hello ;-)

First, thanks for your work! ;-)

We have a problem with your library in a use case with a big graph where the library wrongfully alters structures in the JSON (so differences between what i give in and what comes out after apply()) - sadly, even stuff not even explicitly targeted by the operations get changed.

To show the wrong behavior, I created a test in my branch https://github.com/narcoticfresh/php-jsonpatch/tree/feature/emptyobject-conversion in commit narcoticfresh@c80a98b

Consider this document and operation:

$targetDocument = '{"foo":{"obj": {"property": "value", "emptyObj": {}}}}';
$patchDocument = '[{"op":"add", "path":"/foo/obj/anotherProp", "value":"qux"}]';
$expectedDocument = '{"foo":{"obj": {"property": "value", "anotherProp": "qux", "emptyObj": {}}}}';

The unit test will fail with:

1) Rs\Json\PatchAddTest::shouldAddEmptyObject
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 stdClass Object (
     'foo' => stdClass Object (
         'obj' => stdClass Object (
             'property' => 'value'
             'anotherProp' => 'qux'
-            'emptyObj' => stdClass Object ()
+            'emptyObj' => Array ()
         )
     )
 )

As you can see, emptyObj is converted from \stdClass to array. This is clearly a wrong behavior.

It's obvious this problem comes from the fact that the json is decoded to array in the Operation, then json_encode()d again later - it's an old problem of PHP that {} becomes an array in php, so on re-encoding it will wrongfully be []..

Do you acknowledge this as bug? What could be a feasible way to correct it?

Thanks

[narcoticfresh]

@raphaelstolt it could do a PR if we can find a way to correct it..

Could it be an option to wrap the payload in an \ArrayAccess instance, have the access logic there (handling numeric indexes on array as opposed to string indexes on objects)?

Have the payload there as json_encode($arr, false) (so as object, not array) - then we can encode it back at the end and it should be fine..

[raphaelstolt]

Yes that would be probably an approach. Feel free to send a PR (as I won't have time to look into this issue in the near future) and I'll merge it in time.

[narcoticfresh]

@raphaelstolt

I opened PR #29, seems to work nicely..

Sadly an approach with \ArrayAccess wasn't feasible as the byRef stuff that is used in the add operation doesn't go well with \ArrayAccess - so to not have to do a complete rewrite, I just decided for that easy approach..

What you think? If it can be merged, I would appreciate a tag afterwards so we can update our dependencies ;-)