groc-prog / pyneo4j-ogm

A asynchronous Object-Graph-Mapper for Neo4j 5+ and Python 3.10+
MIT License
12 stars 1 forks source link

Polymorphism not working in relationships #1

Closed agnoam closed 7 months ago

agnoam commented 7 months ago

Hey, I have tried to implement multiple classes that inherit one from another and tried to connect them by relationships. package version: >=0.4.0

The example below:

class VehicleNode(NodeModel):
    engine: str

class CarNode(VehicleNode):
    has_windows: bool

class Consumed(RelationshipModel):
    liters_consumed: int

class GasStationNode(NodeModel):
    volume_in_litres: int
    clients: RelationshipProperty[
        VehicleNode, Consumed
    ] = RelationshipProperty(
        target_model=VehicleNode,
        relationship_model=Consumed,
        direction=RelationshipPropertyDirection.OUTGOING,
        cardinality=RelationshipPropertyCardinality.ZERO_OR_MORE,
        allow_multiple=True
    )

I tried to connect a CarNode and a GasStationNode with a basic relationship like the example below:

client: Pyneo4jClient # Already initialized
await client.register_models([VehicleNode, CarNode, GasStationNode, Consumed])

car = await CarNode(engine='v8', has_windows=True).create()
gas_station = await GasStationNode(volume_in_litres=6000).create()

# Trying to connect them by basic relationship
await gas_station.clients.connect(car, { 'liters_consumed': 50 })

The operation failed with an exception. Exception print:

self = RelationshipProperty(target_model_name=VehicleNode, relationship_model=Consumed, direction=OUTGOING, cardinality=ZERO_OR_MORE, allow_multiple=True)
nodes = CarNode(element_id=4:218fdb95-3060-4d65-ab59-de3bc9a5722f:1, destroyed=False)

    def _ensure_alive(self, nodes: Union[T, List[T]]) -> None:
        """
        Ensures that the provided nodes are alive.

        Args:
            nodes (T | List[T]): Nodes to check for hydration and alive.

        Raises:
            InstanceNotHydrated: Raised if a node is not hydrated yet.
            InstanceDestroyed: Raised if a node has been marked as destroyed.
        """
        nodes_to_check = nodes if isinstance(nodes, list) else [nodes]

        for node in nodes_to_check:
            logger.debug(
                "Checking if node %s is alive and of correct type",
                getattr(node, "_element_id", None),
            )
            if getattr(node, "_element_id", None) is None or getattr(node, "_id", None) is None:
                raise InstanceNotHydrated()

            if getattr(node, "_destroyed", True):
                raise InstanceDestroyed()

            if cast(Type[T], self._target_model).__name__ != node.__class__.__name__:
>               raise InvalidTargetNode(
                    expected_type=cast(Type[T], self._target_model).__name__,
                    actual_type=node.__class__.__name__,
                )
E               pyneo4j_ogm.exceptions.InvalidTargetNode: Expected target node to be of type VehicleNode, but got CarNode
groc-prog commented 7 months ago

Hey! Polymorphism (and inheritance in general) has fallen a bit short since I decided to add support for pydantic v2 (which actually took a lot more time than expected), but was actually planned from the beginning (and will definitely be added in the future).

The problem you are encountering is due to the fact that the validity of relationships is being checked by comparing the actual name of the class passed as a target model, rather than the labels defined for said model. This is also how models are currently resolved by the client. There are also some other known issues, for example the way settings should actually behave when inheriting from another class rather than overwriting everything. This will change in the future to use the labels/type defined for the model instead of the class name, but this comes with some refactoring, which in turn will need a lot of testing to make sure everything still works as expected.

I'm currently working on a new release and I might as well try and squeeze in said fixes, but I can't promise anything since I don't really have that much time to work on this project accept for the weekends (and maybe some evenings), but I will keep you posted on the progress.

groc-prog commented 7 months ago

Hey, just following up on this.

Version 0.5.0 has just been release which should have introduces polymorphism, together with a new migration cli. If you find any more bugs or have any feature requests, you are welcome to open up another issue! I am closing this issue now since it seems to be resolved.