graphaware / neo4j-bolt-php

PHP Driver for Neo4j's Binary Protocol : Bolt
MIT License
42 stars 38 forks source link

Problem with empty arrays used in properties #22

Closed sgehrig closed 7 years ago

sgehrig commented 7 years ago

When serializing the following properties, the query fails:

$properties = [
    'id' => 'whatever',
    'name' => 'some name',
    'tags' => []
 ];

This is due to a problem in GraphAware\Bolt\PackStream\Packer::pack(). If the method encounters a PHP array it uses the following code to determine whether it should serialize the data to a Bolt list or to a Bolt map.

$stream .= ($this->isList($v) && !empty($v)) ? $this->packList($v) : $this->packMap($v);

Because the array is empty the condition falls into the else branch and tries to pack the value as a Bolt map. But maps are not allowed as property values.

Property values can only be of primitive types or arrays thereof    

I'm not 100% sure but I assume, the empty check here is unnecessary, isn't it? Or are there some Bolt restrictions with empty lists?

ikwattro commented 7 years ago

The problem is in PHP there is no way to define if an empty array should be an empty List or empty Map. I'll have to add custom classes for defining it and you'll need to use it yourself (so making the check if empty create an empty map class or list class).

sgehrig commented 7 years ago

That's true. Can't you track the nesting depth inside the packer? Serialize an empty array to a map on the first level and to a list otherwise.

Perhaps I'm just thinking too simple here.

ikwattro commented 7 years ago

problem is you can have both in any level depending on your query unfortunately :(

sgehrig commented 7 years ago

I see. You're right - you'd need special types on the PHP side to be able to differentiate the two. From user land code a developer should be able to manually select the right behavior I assume.

Do you have any plans to add this?

ikwattro commented 7 years ago

I will add them pretty soon yes, been mainly busy on other stuff. Stay tuned.

ikwattro commented 7 years ago

Hi @sgehrig Fixed in #23 . New release in a couple of minutes

sgehrig commented 7 years ago

Awesome! Thx!