neo4j-contrib / neomodel

An Object Graph Mapper (OGM) for the Neo4j graph database.
https://neomodel.readthedocs.io
MIT License
936 stars 231 forks source link

Relationship properties marked with db_property are skipped during inflation #774

Closed j-krose closed 4 months ago

j-krose commented 8 months ago

Expected Behavior

Properties of StructuredRel relationships that are marked with a db_property property name override are properly written to the database but skipped during inflation, and thus are always filled with None when returned from queries.

Actual Behavior

Properties of StructuredRel relationships that are marked with a db_property property name override should be filled from their value in the database during inflation.

How to Reproduce the Problem

Create a StructuredRel with a property that uses db_property and attempt to round-trip the relationship to the database.

Simple Example

Simple reproduction using slightly modified versions of objects from the example code:

import neomodel
from neomodel import StringProperty, StructuredRel, StructuredNode, RelationshipTo, IntegerProperty

class FriendRel(StructuredRel):
    property_one = IntegerProperty()
    propertyTwo = IntegerProperty()
    # This is a pattern I expect to be common in my workflow because python member variables are snake_case by
    # convention but neo4j properties are camelCase by convention
    # (https://neo4j.com/docs/getting-started/appendix/graphdb-concepts/#graphdb-naming-conventions)
    property_three = IntegerProperty(db_property="propertyThree")

class Person(StructuredNode):
    name = StringProperty()
    friends = RelationshipTo("Person", "FRIEND", model=FriendRel)

if __name__ == "__main__":
    neomodel.config.DATABASE_URL = "bolt://<user>:<pass>@localhost:<port>"

    jim = Person(name="Jim").save()
    bob = Person(name="Bob").save()
    rel = jim.friends.connect(bob, {"property_one": 1, "propertyTwo": 2, "property_three": 3})
    print(f"property_one: {rel.property_one}, propertyTwo: {rel.propertyTwo}, property_three: {rel.property_three}")

prints:

property_one: 1, propertyTwo: 2, property_three: None

The values do properly make it to the database:

Screen Shot 2023-12-31 at 12 14 21

And during inflation, the value is present in the _properties dict of the db object:

Screen Shot 2023-12-31 at 12 19 24

Cause

This seems to be caused by StructuredRel.inflate(...) not considering the db_property field, and only checking the python key of the property:

class StructuredRel(StructuredRelBase):
    ...

    @classmethod
    def inflate(cls, rel):
        """
        Inflate a neo4j_driver relationship object to a neomodel object
        :param rel:
        :return: StructuredRel
        """
        props = {}
        for key, prop in cls.defined_properties(aliases=False, rels=False).items():
            if key in rel:
                props[key] = prop.inflate(rel[key], obj=rel)
            elif prop.has_default:
                props[key] = prop.default_value()
            else:
                props[key] = None
        srel = cls(**props)
        srel._start_node_element_id_property = rel.start_node.element_id
        srel._end_node_element_id_property = rel.end_node.element_id
        srel.element_id_property = rel.element_id
        return srel

whereas StructuredNode.inflate(...) gives the db_property priority over the key:

class StructuredNode(NodeBase):
    ...

    @classmethod
    def inflate(cls, node):
        """
        Inflate a raw neo4j_driver node to a neomodel node
        :param node:
        :return: node object
        """
        # support lazy loading
        if isinstance(node, str) or isinstance(node, int):
            snode = cls()
            snode.element_id_property = node
        else:
            node_properties = _get_node_properties(node)
            props = {}
            for key, prop in cls.__all_properties__:
                # map property name from database to object property
                db_property = prop.db_property or key

                if db_property in node_properties:
                    props[key] = prop.inflate(node_properties[db_property], node)
                elif prop.has_default:
                    props[key] = prop.default_value()
                else:
                    props[key] = None

            snode = cls(**props)
            snode.element_id_property = node.element_id

        return snode

    ...

Specifications

Versions

j-krose commented 8 months ago

FYI I am putting together a PR to fix this. It looks like SemiStructuredNode also has the same problem. I will fix both.