From 5763c5e0baf690cc046596ac03a66a2a29d9ff47 Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Sun, 18 Dec 2022 15:13:21 +0100 Subject: [PATCH] Don't use the entities cache when upserting entities. This may make things a bit less optimal, but it's probably the only possible solution that preserves my sanity. Managing upserts of cached instances that were previously made transient and expunged from the session is far from easy, and the management of recursive parent/children relationships only add one more layer of complexity (and that management is already complex enough in its current implementation). --- platypush/entities/_engine/repo/__init__.py | 46 ++++++++++----------- platypush/entities/_engine/repo/merger.py | 4 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/platypush/entities/_engine/repo/__init__.py b/platypush/entities/_engine/repo/__init__.py index 8fdab4db9..ed355f341 100644 --- a/platypush/entities/_engine/repo/__init__.py +++ b/platypush/entities/_engine/repo/__init__.py @@ -37,40 +37,40 @@ class EntitiesRepository: logger.info('Entities cache initialized') def get( - self, session: Session, entities: Iterable[Entity] + self, session: Session, entities: Iterable[Entity], use_cache=True ) -> Dict[Tuple[str, str], Entity]: """ Given a set of entity objects, it returns those that already exist (or have the same ``entity_key``). It looks up both the cache and the database. """ - entities_map: Dict[Tuple[str, str], Entity] = { - e.entity_key: e for e in entities - } - - # Fetch the entities that exist in the cache existing_entities = {} - # TODO UNCOMMENT THIS CODE TO ACTUALLY USE THE CACHE! - # existing_entities = { - # key: self._entities_cache.by_external_id_and_plugin[key] - # for key in entities_map.keys() - # if key in self._entities_cache.by_external_id_and_plugin - # } + if not use_cache: + existing_entities = self._db.fetch(session, entities) + self._cache.update(*existing_entities.values()) + else: + # Fetch the entities that exist in the cache + existing_entities = { + e.entity_key: self._cache.get(e) for e in entities if self._cache.get(e) + } - # Retrieve from the database the entities that miss from the cache - cache_miss_entities = { - key: e for key, e in entities_map.items() if key not in existing_entities - } + # Retrieve from the database the entities that miss from the cache + cache_miss_entities = { + e.entity_key: e + for e in entities + if e.entity_key not in existing_entities + } - cache_miss_existing_entities = self._db.fetch( - session, cache_miss_entities.values() - ) + cache_miss_existing_entities = self._db.fetch( + session, cache_miss_entities.values() + ) - # Update the cache - self._cache.update(*cache_miss_existing_entities.values()) + # Update the cache + self._cache.update(*cache_miss_existing_entities.values()) + + # Return the union of the cached + retrieved entities + existing_entities.update(cache_miss_existing_entities) - # Return the union of the cached + retrieved entities - existing_entities.update(cache_miss_existing_entities) return existing_entities def save(self, *entities: Entity) -> Iterable[Entity]: diff --git a/platypush/entities/_engine/repo/merger.py b/platypush/entities/_engine/repo/merger.py index 9b18ec85a..cb397c268 100644 --- a/platypush/entities/_engine/repo/merger.py +++ b/platypush/entities/_engine/repo/merger.py @@ -49,7 +49,7 @@ class EntitiesMerger: (Recursive) inner implementation of the entity merge logic. """ processed_entities = [] - existing_entities.update(self._repo.get(session, entities)) + existing_entities.update(self._repo.get(session, entities, use_cache=False)) # Retrieve existing records and merge them for entity in entities: @@ -96,7 +96,7 @@ class EntitiesMerger: # If there's no parent_id but there is a parent object, try to fetch # its stored version if not parent_id and parent: - batch = list(self._repo.get(session, [parent]).values()) + batch = list(self._repo.get(session, [parent], use_cache=False).values()) # If the parent is already stored, use its ID if batch: