Always read an entity's parent through get_parent when climbing up.

This should avoid the risk of `DetachedInstanceError` by retrieving the
object into the session if it's not available.
This commit is contained in:
Fabio Manganiello 2023-03-22 22:41:09 +01:00
parent 5c23d3aa87
commit e71c312133
Signed by: blacklight
GPG key ID: D90FBA7F76362774
4 changed files with 31 additions and 27 deletions

View file

@ -4,9 +4,8 @@ from typing import Dict, Iterable, Optional, Tuple
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
from platypush.entities._base import Entity, EntityMapping from platypush.entities._base import Entity, EntityMapping
# pylint: disable=no-name-in-module
from platypush.entities._engine.repo.db import EntitiesDb from platypush.entities._engine.repo.db import EntitiesDb
from platypush.entities._engine.repo.helpers import get_parent
from platypush.entities._engine.repo.merger import EntitiesMerger from platypush.entities._engine.repo.merger import EntitiesMerger
logger = logging.getLogger('entities') logger = logging.getLogger('entities')
@ -98,7 +97,7 @@ class EntitiesRepository:
""" """
parent = entity parent = entity
while parent: while parent:
parent = self._merge.get_parent(session, entity) parent = get_parent(session, entity)
if parent: if parent:
entity = parent entity = parent

View file

@ -7,6 +7,7 @@ from sqlalchemy.orm import Session
from platypush.context import get_plugin from platypush.context import get_plugin
from platypush.entities._base import Entity from platypush.entities._base import Entity
from .helpers import get_parent
@dataclass @dataclass
@ -69,7 +70,7 @@ class EntitiesDb:
batch.clear() batch.clear()
def _split_entity_batches_for_flush( def _split_entity_batches_for_flush(
self, entities: Iterable[Entity] self, session: Session, entities: Iterable[Entity]
) -> List[List[Entity]]: ) -> List[List[Entity]]:
""" """
This method retrieves the root entities given a list of entities and This method retrieves the root entities given a list of entities and
@ -93,9 +94,10 @@ class EntitiesDb:
for entity in entities: for entity in entities:
parent_key = None parent_key = None
parent_id = entity.parent_id parent_id = entity.parent_id
if entity.parent: parent = get_parent(session, entity)
parent_id = parent_id or entity.parent.id if parent:
parent_key = entity.parent.entity_key parent_id = parent_id or parent.id
parent_key = parent.entity_key
if parent_id: if parent_id:
children_by_parent_id[parent_id][entity.entity_key] = entity children_by_parent_id[parent_id][entity.entity_key] = entity
@ -169,7 +171,7 @@ class EntitiesDb:
Persist a set of entities. Persist a set of entities.
""" """
# Get the "unwrapped" batches # Get the "unwrapped" batches
batches = self._split_entity_batches_for_flush(entities) batches = self._split_entity_batches_for_flush(session, entities)
# Flush each batch as we process it # Flush each batch as we process it
for batch in batches: for batch in batches:

View file

@ -0,0 +1,18 @@
from typing import Optional
from sqlalchemy.orm import Session, exc
from platypush.entities import Entity
def get_parent(session: Session, entity: Entity) -> Optional[Entity]:
"""
Gets the parent of an entity, and it fetches if it's not available in
the current session.
"""
try:
return entity.parent
except exc.DetachedInstanceError:
# Dirty fix for `Parent instance <...> is not bound to a Session;
# lazy load operation of attribute 'parent' cannot proceed`
return session.query(Entity).get(entity.parent_id) if entity.parent_id else None

View file

@ -1,9 +1,11 @@
from typing import Iterable, List, Optional from typing import Iterable, List, Optional
from sqlalchemy.orm import Session, exc from sqlalchemy.orm import Session
from platypush.entities._base import Entity, EntityMapping from platypush.entities._base import Entity, EntityMapping
from .helpers import get_parent
# pylint: disable=too-few-public-methods # pylint: disable=too-few-public-methods
class EntitiesMerger: class EntitiesMerger:
@ -93,7 +95,7 @@ class EntitiesMerger:
appropriately rewired and that all the relevant objects are added to appropriately rewired and that all the relevant objects are added to
this session. this session.
""" """
parent = cls.get_parent(session, entity) parent = get_parent(session, entity)
if not parent: if not parent:
# No parent -> we can terminate the recursive climbing # No parent -> we can terminate the recursive climbing
return entity return entity
@ -140,23 +142,6 @@ class EntitiesMerger:
cls._sync_parent(session, existing_parent, new_entities, existing_entities) cls._sync_parent(session, existing_parent, new_entities, existing_entities)
return entity return entity
@staticmethod
def get_parent(session: Session, entity: Entity) -> Optional[Entity]:
"""
Gets the parent of an entity, and it fetches if it's not available in
the current session.
"""
try:
return entity.parent
except exc.DetachedInstanceError:
# Dirty fix for `Parent instance <...> is not bound to a Session;
# lazy load operation of attribute 'parent' cannot proceed`
return (
session.query(Entity).get(entity.parent_id)
if entity.parent_id
else None
)
@staticmethod @staticmethod
def _append_children(entity: Entity, *children: Entity): def _append_children(entity: Entity, *children: Entity):
""" """