Vinelab / NeoEloquent

The Neo4j OGM for Laravel
MIT License
633 stars 199 forks source link

Setting relationships when using custom primaryKey #196

Closed guivazcabral closed 7 years ago

guivazcabral commented 7 years ago

Hi. I'm using custom primary keys named 'uuid' on my models Author and Book. Each model has protected $primaryKey = 'uuid'; public $incrementing = false; I want to create a relationship from Author to Book, labeled :WRITE

$book->setAuthor()->associate($author)->save()

my setAuthor() is this: public function setAuthor(){ return $this->belongsTo('\App\Http\Model\Author', 'WRITE'); }

and is returning me Exception in GetNodeRelationships.php line 73: No node id specified

can someone help me?

KinaneD commented 7 years ago

@guivazcabral Which version of NE are you using? associate is meant to be used with one-to-one relations, in this case you have a one-to-many relation, try using attach instead.

guivazcabral commented 7 years ago

Hi @KinaneD

$ composer show -i | grep neoeloquent vinelab/neoeloquent v1.4.2

I haven't tried to use attach yet.

guivazcabral commented 7 years ago

@KinaneD

Hi

With attach it returns

Exception in CreateRelationship.php line 77: No relationship start node specified

KinaneD commented 7 years ago

Please make sure when you create and query a node it actually has a uuid property. If it doesn't please update to NE 1.5-dev, we are using it in production so no worries on it being stable.

guivazcabral commented 7 years ago

@KinaneD do you mean version "vinelab/neoeloquent": "1.4.*-dev", ? tried update to 1.5-dev and it didn't work

anyway, updated to 1.4*-dev and it still returns Exception in CreateRelationship.php line 77: No relationship start node specified

i leave here my models: book author

PS: yes, all the nodes actually have uuid properties

KinaneD commented 7 years ago

@guivazcabral I need to see more details, please share a snippet of how you use the attach/associate or any of the other way you use to create the relation. I do not know of a CreateRelationship.php class in NE, nor the exception message sound familiar, it must be something from your side, is it?

guivazcabral commented 7 years ago

@KinaneD You can checkout my code at https://github.com/guivazcabral/api/tree/master/app/Http

The class CreateRelationship.php is on vendor/heydavid713/neo4jphp/lib/Everyman/Neo4j/Command/CreateRelationship.php

I know that's another package and shouldn't be NE responsability, but if I remove my custom primaryKey it works as it should.

Thanks for your attention.

KinaneD commented 7 years ago

Weird, i can't seem to find a reason why it is not working. Did you try using the Author's id instead of the model with the attach()? Try using save(). @Mulkave Any idea what this is all about?

guivazcabral commented 7 years ago

Hi @KinaneD

tried with the author's id in the attach

$book->setAuthor()->attach($author->uuid);

and with save

$book->setAuthor()->attach($author->uuid)->save();

i still get the same error.

I forgot to mention that i'm using PHP 7.0.8, I don't know if it could be the cause of the problem

KinaneD commented 7 years ago

With save i meant $book->setAuthor()->save($author);, sorry for the misunderstanding.

guivazcabral commented 7 years ago

@KinaneD

doesn't work, i get the same exception

gemini-git commented 7 years ago

I have the same issue and I think that I found the problem.

Every relationship node will be created by the asNode method of the Vinelab\NeoEloquent\Eloquent\Edges\Delegate class with the following code

public function asNode(Model $model) {
    $node = $this->client->makeNode();

    // If the key name of the model is 'id' we will need to set it properly with setId()
    // since setting it as a regular property with setProperty() won't cut it.
    if ($model->getKeyName() == 'id') {
        $node->setId($model->getKey());
    }

    // In this case the dev has chosen a different primary key
    // so we use it insetead.
    else {
        $node->setProperty($model->getKeyName(), $model->getKey());
    }

    return $node;
}

As you can see, only the original id key will be set by the setId method.

When you take a look to the Everyman\Neo4j\Command\CreateRelationship class, the exception will be thrown in thegetPath method:

protected function getPath() {
    $start = $this->rel->getStartNode();
        if (!$start || !$start->hasId()) {
            throw new Exception('No relationship start node specified');
        }
    return '/node/'.$start->getId().'/relationships';
 }

The method checks whether the node has an id ($this->id !== null). But with own primary keys id is null => Exception

guivazcabral commented 7 years ago

ping @KinaneD

ctorresr commented 7 years ago

Is not recommended to mess with node ids. You need an attribute on the node for uuid. After that you can search for a node using the uuid, and save or attach using the objects.

$book = \Book::whereUuid('496dc4df-58d0-4879-b260-c919145a24b')->first()

$author = \Author::whereUuid('523154f7-9346-4cf8-b841-b7be1b93fad')->first()

$book->setAuthor()->attach($author);

or depending if edge in or edge out

$book->setAuthor()->save($author);

This will give you all relations for an author with multiple books using the relation from the author side

$author->nameOfYourRelationFromAuthorToBooks()->edge()

You can then load a relation to delete if needed.

$relation = $author->nameOfYourRelationFromAuthorToBooks()->edge($book)
$relation->delete();

Hopes this helps.

KinaneD commented 7 years ago

@guivazcabral sorry to find that your issue remains. As @ctorresr mentioned, changing the internal Laravel id is not what you should be looking into. I am not sure if using php 7.* can have any effect on the desired behaviour. The implementation referred to by @geminixxl is of an older NE release and of an older driver Everyman. I recommend that you update to NeoEloquen 1.5-dev as i mentioned previously which runs on a different driver neoclient. We have it running on production and we also use uuid with no issues. I just noticed the title of this issue refers to relations, i would like to clarify that NE doesn't support custom ids on relationships, just in case it is something you might be looking for.

ctorresr commented 7 years ago

@KinaneD php 7* is not the culprit, we have php 7 and NE 1.4 with the @heydavid713 neo4jphp fork of Everyman running on production without problems.

For me, It's seems they are treating neo4j as a relational db.

heydavid713 commented 7 years ago

As @KinaneD and @ctorresr mentioned it. Relations don't support custom keys on Everyman driver.

guivazcabral commented 7 years ago

@KinaneD

the solution @ctorresr mentions seems to be apropriate for my case, although I'm not able to put it to use right now.

i cant seem to be able to update to NE 1.5-dev. care to enlight me how should I do it? thanks everyone for the replies :)

guivazcabral commented 7 years ago

Another quick question. Sorry for not opening another issue.

How do I change the created_at and updated_at parameters to timestamps?

heydavid713 commented 7 years ago

I think you just change the $dateFormat property but I'm not entirely sure if that's the name, but it is along those lines.

On Nov 18, 2016 6:57 PM, "Guilherme Cabral" notifications@github.com wrote:

Another quick question. Sorry for not opening another issue.

How do I change the created_at and updated_at parameters to timestamps?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Vinelab/NeoEloquent/issues/196#issuecomment-261664108, or mute the thread https://github.com/notifications/unsubscribe-auth/ALGPKRU_OQ7xnc38hZLWL9eCZWDAy6CLks5q_i1ngaJpZM4KlqjR .

KinaneD commented 7 years ago

@guivazcabral require "vinelab/neoeloquent": "dev-1.5-dev" and update composer.json. Formatting dates is done by either setting the default date format like in here: https://laravel.com/docs/5.3/eloquent-mutators#date-mutators, or by using accessors and mutators like in here: http://stackoverflow.com/questions/24441395/how-to-change-default-format-at-created-at-and-updated-at-value-laravel.

gemini-git commented 7 years ago

It looks like 1.5-dev does not work with laravel 5.3, because I get the following error:

Call to undefined method Illuminate\Foundation\Application::bindShared()

KinaneD commented 7 years ago

@geminixxl That is probably due to the service provider that has changed, i am sorry that the docs doesn't mention such change. Please use the following service provider instead Vinelab\NeoEloquent\Frameworks\Laravel\NeoEloquentServiceProvider52::class. Please try it and get back to us.

gemini-git commented 7 years ago

The change works but there are more problems with 1.5-dev and laravel 5.3. I opened a new issue #205

KinaneD commented 7 years ago

@guivazcabral Did you manage to solve this?

guivazcabral commented 7 years ago

Hi. Ended up @ctorresr solution. Haven't still tried to change the dateFormat as you suggested but I'll try it later. Thank for your support, you can close the issue :)

Mulkave commented 7 years ago

Good to hear 👍

Binternet commented 1 year ago

Faced this issue as well When changed the primaryKey attribute to be a none-integer one and then tried to create a relationship, I got:

Laudis\Neo4j\Types\Node::__construct(): Argument #1 ($id) must be of type int

From the neo4j driver So I removed the primaryKey attribute and added a custom lookup such as findById