arangodb / arangodb-php

PHP ODM for ArangoDB
https://www.arangodb.com
Apache License 2.0
182 stars 46 forks source link

Solves json_encode problem. When an external thing (like a script or … #206

Closed evaldobarbosa closed 7 years ago

evaldobarbosa commented 7 years ago

…framework) not calling toJson method, json is returned empty

frankmayer commented 7 years ago

That's strange. When you do (string) $documentObject, the toJson() method is called automatically by the __toString() method and should return valid JSON (I have also added an appropriate test, here: https://github.com/arangodb/arangodb-php/blob/1ebef038f1a8fc3e31fce8f0e2b311ac97fca7e4/tests/DocumentBasicTest.php#L473).

Could you elaborate on your scenario?

evaldobarbosa commented 7 years ago

I am using Silex framework and retrieving information from arango to a Web API. When I send my resultset to the JsonResponse, the data is missing, all objects seem empty. See the example:

I have three documents. When I try json_encode($cursor->getAll(), the response is: [{},{},{}].

Changing the Document class and implementing JsonSerializable, this problem is solved.

frankmayer commented 7 years ago

Oh, I see. Yes. From a quick look at Silex's JsonResponse class, it expects an array or an object which would provide its data as public properties. Since the document object does not have any public properties (as the data is stored inside a protected one) the result-set is comprised of empty JSON objects.

@jsteemann It seems that indeed implementing JsonSerializable is the easiest way to go, here. It does add ~70% processing time, which is only noticeable in a tight loop, though. The other (performance wise) better way to go, would probably be, to be able to return result-set documents as arrays instead of creating document objects, for this kind of cases. That way, performance would be even better, as we don't have the overhead of creating those unneeded documents in the first place. What do you think?

jsteemann commented 7 years ago

@frankmayer: it would be quite nice if one could configure the driver to return either Document objects or plain PHP arrays/objects or something else. Then every user could pick the format that best suitable for their case. Users that higher performance could go with plain PHP types and others could go with more heavy-weight Document objects. I think this would be quite some work, and at least all the places that call Document::createFromArray() would need adjustment. You know the driver internals better than I do. Do you think that would be possible with reasonable effort?

frankmayer commented 7 years ago

@jsteemann I think, it would be doable with reasonable effort. I'll have a look and get back.

evaldobarbosa commented 7 years ago

@jsteemann, I think JsonSerializable should be implemented as a first solution and a better solution can come after. The @frankmayer's idea is good: choose a parameter that changes the way of return data (documents or array). Anyway, jsonSerialize returns Cursor :: getAll (). This method could be enhanced to attend this.

frankmayer commented 7 years ago

It turns out, there was already an option built-in for this.

@evaldobarbosa If you take a look at this test (https://github.com/arangodb/arangodb-php/blob/devel/tests/CollectionExtendedTest.php#L797), you will see that passing the option '_flat' => true in the options array of whatever call you are making, will then return its result-set as an array when you call $cursor->getAll(). I believe this is the thing you actually need, and this way there is neither the performance penalty of implementing JsonSerializable nor the one of having to instantiate document objects, when you only need arrays to be returned. Let me know if this works for you.

Please understand that I am a bit cautious in changing this kind of things (like implementing the JsonSerializable interface), especially when they have a performance impact, because this is a driver.

evaldobarbosa commented 7 years ago

@frankmayer, see this example.

https://github.com/evaldobarbosa/arangodb-php-pr-example

frankmayer commented 7 years ago

@evaldobarbosa ok, installation took a bit longer as I had to first create the config.json in the private folder ;) So, I created a document with the data of the example your are mentioning in your IndexController and it does work, as described. By localhost:9000 I do get this response:

[{"_key":"1380048022","_id":"noticias\/1380048022","_rev":"_UhfwhmC---","someOtherAttribute":"someOtherValue"}]

Maybe you have some different code in your actual setup, where id doesn't work?

frankmayer commented 7 years ago

@evaldobarbosa BTW, this is what I got from getting all documents:

[{"someOtherAttribute":"someOtherValue","_key":"1380048022"},{"name":"Frank","_key":"1380047806"}]