Cant retrieve additional relations using `fetch_relations`

[diegopso]

Expected Behavior (Mandatory)

I expect that I can define models without defining StructuredRel and use the method fetch_relations to retrieve relationships of my model as described in https://neomodel.readthedocs.io/en/latest/getting_started.html#retrieving-additional-relations

Actual Behavior (Mandatory)

An exception is thrown:

neomodel.exceptions.RelationshipClassNotDefined: Relationship of type IS_FROM does not resolve to any of the known objects

How to Reproduce the Problem

Simple Example

For instance, the code below from docs (with adjusts) should work given the correct DB URL.

from neomodel import (
    IntegerProperty,
    RelationshipFrom,
    RelationshipTo,
    StringProperty,
    StructuredNode,
    UniqueIdProperty,
    config,
)

config.DATABASE_URL = "bolt://***"


class Country(StructuredNode):
    code = StringProperty(unique_index=True, required=True)
    inhabitant = RelationshipFrom("Person", "IS_FROM")


class City(StructuredNode):
    name = StringProperty(required=True)
    country = RelationshipTo(Country, "FROM_COUNTRY")


class Person(StructuredNode):
    uid = UniqueIdProperty()
    name = StringProperty(unique_index=True)
    age = IntegerProperty(index=True, default=0)

    # traverse outgoing IS_FROM relations, inflate to Country objects
    country = RelationshipTo(Country, "IS_FROM")

    # traverse outgoing LIVES_IN relations, inflate to City objects
    city = RelationshipTo(City, "LIVES_IN")


jim = Person(name="Jim", age=3).save()  # Create
jim.age = 4
jim.save()  # Update, (with validation)

germany = Country(code="DE").save()
jim.country.connect(germany)
berlin = City(name="Berlin").save()
berlin.country.connect(germany)
jim.city.connect(berlin)

results = Person.nodes.fetch_relations("country").all()
for result in results:
    print(result[0])  # Person
    print(result[1])  # associated Country

To make it work at least to the point of the issue I have, I added the inhabitant relation in country and changed the order of invocation of methods all and fetch_relations .

Work around

If I manually define the model of the relationship to be StructuredRel classes, the error is not triggered.

Adding:

from neomodel import StructuredRel

And changing the relation to:

inhabitant = RelationshipFrom("Person", "IS_FROM", model=StructuredRel)

Results in:

Although this works, it brings some weird trace info if looking in a debugger:

Screenshots (where it's possibile)

The error on my terminal:

Specifications (Mandatory)

Currently used versions

Versions

• OS: Ubuntu 22.04.2 LTS (on WSL2)
• Library: 5.3.0
• Neo4j: 5.17.0

[mariusconjeaud]

@tonioo or @owinogradow, do you think you could have a look ? fetch_relations is something we merged back from the NN fork right ?

[mariusconjeaud]

@diegopso Coming back to this, I think that :

• The example in the documentation is wrong indeed => if you look at the tests, in test_match_api.py::test_fetch_relations, you will find several examples that work
• The traceback you see in the id property, is because Neo4j 4 used id as its internal identifier, and Neo4j 5 now uses element_id; but since we still support Neo4j 4, neomodel has to keep both in the node definition. It will then use the right id/element_id depending on the version of the Neo4j server you are connected to (so in your case, element_id). So you can ignore that traceback.

Note on id/element_id: we do provide the possibility to users to use id and element_id in whatever way they want, but we align with the Neo4j guideline, which is that those are internal ids that should only be used by the database itself; they are not guaranteed to be immutable. To identify your nodes in a robust way, you should apply your own unique identifier system.

[mariusconjeaud]

OK, I compared your example with the one in the documentation, and I can confirm that the problem is "just" that the documentation was very bad on that one. Sorry about that.

I will update accordingly, and before I do that, here is what is wrong/missing in the documentation:

• You have to do MyNode.nodes.fetch_relations('myrel__myotherrel').all() in this order, and not all before fetch_relations
• You must give a model to any Relationship object you intend to traverse in such a way, even if you only give it the parent one StructuredRel. That is because fetch_relations will always try to do the object resolution, as it does when running db.cypher_query('MATCH (n) RETURN n', resolve_objects=true) ; and that relies on the NODE_CLASS_REGISTRY object, in which relationships are added only if they have a model attached to them. So in this case, "IS_FROM" does not have a model => not in the node class registry => neomodel does not know what object to resolve it into

Sorry for those two oversights; we had to integrate this back from an end-user branch, and it required quite some structural changes, and so we missed a few points (in that branch, which is actually a full application, relationships always have a model attached to them, so I missed that this was required)

[mariusconjeaud]

@diegopso Please check the linked PR. It does not change the behaviour (because it makes sense that neomodel expects a model to be defined for a relationship it is trying to resolve into a model object); but it fixes and improves the documentation, as well as the content of the exception, so it is clearer what is expected.

Thanks a ton for raising this up ! This is the latest big feature (except for async), and I feel we never truly completed it. That part definitely deserves more attention (it makes neomodel way more graphy after all)

[diegopso]

@mariusconjeaud thank you for the updates. I think now the docs look good.

I agree that a model for a relationship should be defined to be resolved. Still, when implementing a workaround on my side, I extended RelationshipTo and RelationshipFrom to change the default model argument in the init from None to StructuredRel. This, at least for me, made such that there is always a model to resolve the relationship to. This should be enough for all relationships that have no extra properties, and just custom relationships require writing specific classes.

I am mentioning it because you also have something similar for your passing fetch_relations tests in these classes: https://github.com/neo4j-contrib/neomodel/blob/795-cant-retrieve-additional-relations-using-fetch_relations/test/sync_/test_match_api.py#L35-L46

Used in this test case: https://github.com/neo4j-contrib/neomodel/blob/795-cant-retrieve-additional-relations-using-fetch_relations/test/sync_/test_match_api.py#L531

Still, as is now is much easier to understand and use it.

[mariusconjeaud]

Your solution is interesting. I actually thought about that while working on this, but I was looking at a different place where to add StructuredRel as default, and it was not suitable.
But your solution looks better !
It would need testing though, to make sure it's not filling the node class registry with many relationship classes.

I mean that if you define two distinct relationships with the same type string (like, IS_FROM), then it might be trying to create two entries with that key in the registry and trigger a conflict.