Vinelab / NeoEloquent

The Neo4j OGM for Laravel
MIT License
634 stars 200 forks source link

Create method not handling arrays properly #157

Closed DeltaBravo closed 4 years ago

DeltaBravo commented 8 years ago

Consider the following form data:

array [  
    "_token" => "NAt5dIWp1KBzyR4ToEdfeHxl8C2sCl6guNcO6vVx"  
    "nameDisplay" => "ArrayTest"  
    "roleAttributes" => array [  
        0 => "a4a03816-1c25-4e8a-a2aa-459f1f18545c"  
        1 => "f7abdb46-937a-4737-803c-07467e2ef52e"  
    ]  
]

Now we saves a new Role and returns the instance:

class Role extends NeoEloquent {
    public static function create(array $attributes = []) {
        $newRole = parent::create($attributes);

        return $newRole;
    }
}
SyntaxException: Invalid input ''': expected whitespace, comment or a property key name (line 1, column 99 (offset: 98))  
"CREATE (:`Role` { nameDisplay: 'ArrayTest', roleAttributes: 'a4a03816-1c25-4e8a-a2aa-459f1f18545c',  
'f7abdb46-937a-4737-803c-07467e2ef52e'})"  
^

It seems the "roleAttributes" array is being split into strings. The second and subsequent strings then have no property and so the Cypher is invalid.

I understand that Neo4j property arrays only support basic types (number, Boolean, string) and therefore SETting associative or multidimensional arrays cannot be supported by NeoEloquent.

In this case an array ($arguments) is being parsed into property/value pairs which are used to build a Cypher query. It seems to me that where an array element is an array the key should be used as a property (as usual) and the array values should be set as an array that Cypher can use.

I tried converting the array to a string before passing it to NeoEloquent:

$attributes['roleAttributes'] = '[' . implode(',', $attributes['roleAttributes']) . ']';

..but this only results in roleAttributes being a string property:

"roleAttributes": "[a4a03816-1c25-4e8a-a2aa-459f1f18545c,f7abdb46-937a-4737-803c-07467e2ef52e]"

..rather than an array property:

"roleAttributes": ["a4a03816-1c25-4e8a-a2aa-459f1f18545c","f7abdb46-937a-4737-803c-07467e2ef52e"]
KinaneD commented 8 years ago

Hi @DeltaBravo , NeoEloquent doesn't handle setting array properties yet it will be available for the next release. On a side note, there is no need to override the create method, you can do something like this return $this->create($attributes)

DeltaBravo commented 8 years ago

Thank you for responding!

I am overriding the create method for three reasons:

public static function create(array $attributes = []) {
    $attributes['uuid'] = (string)UUID::generate(4);

    // #DRB #REVIEW Vinelab/NeoEloquent 1.3.1 does not seem to properly support creating nodes with array properties
    $roleAttributes = '["' . implode('","', $attributes['roleAttributes']) . '"]';
    unset($attributes['roleAttributes']);

    $newRole = parent::create($attributes);

    $masterRole = session()->get('client')->masterRole;

    DB::connection('neo4j')->select('
        MATCH (mr {uuid:"' . $masterRole . '"}), (new {uuid:"' . $newRole->uuid . '"})
        MERGE (mr)<-[:HAS_ROLE]-(new)
        SET new.roleAttributes = ' . $roleAttributes . ';');

    return $newRole;
}
KinaneD commented 8 years ago

@DeltaBravo For some reason you create method is static, so you would only be able to call the parent's create. I would recommend using the creating event if you need to manipulate the attributes before persisting to neo4j. https://laravel.com/docs/4.2/eloquent#model-events

alegargar commented 5 years ago

Any news about this? Array attributes will be supported or there is any alternative to store an attribute representing a collection or a sub-object?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.