Fix event/catalogue membership ORM bugs and bump 0.5.0 → 0.5.1

Two bugs surfaced when tscat_gui (and SciQLop, via the tscat_gui driver)
moved to 0.5.0 — both visible as ValueError raised from
Backend.{add,remove}_events_to_catalogue:

* remove_events_from_catalogue compared membership by Python id() instead
  of the SQL primary key. Any caller that resolved an Event via
  get_events(uuid=...) (the new fast path that returns wrappers backed
  by _LazyBackendEntity) ended up with a different Python instance from
  the one stored in catalogue.events and got "Event is not in catalogue."
  Fixed by comparing on e.id, matching add_events_to_catalogue.

* After event.remove(permanently=True), catalogue.events kept the
  deleted ORM Event in the in-memory collection. A subsequent
  add_events_to_catalogue() with a freshly-created Event reusing the
  same UUID then hit the existing-id check and was rejected, even
  though tscat.get_events(catalogue) reported the catalogue empty.
  Backend.remove now detaches an Event from every catalogue.events
  collection that holds it before session.delete().

The fast-path query also let _LazyBackendEntity proxies leak into
SQLAlchemy relationships (catalogue.events.extend), which the membership
fix would still trip over later. Added a _resolve_be helper in base.py
and used it at the wrapper→backend boundary for add/remove_events_to_catalogue
and _BackendBasedEntity.remove so SQLAlchemy only ever sees identity-mapped
ORM entities.

Two regression tests cover both scenarios.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: jeandet

pyproject.toml

@@ -4,7 +4,7 @@ requires = ["hatchling"]
 
 [project]
 name = "tscat"
-version = "0.5.0"
+version = "0.5.1"
 description = "A library which stores, loads and filters time-series-events and catalogues."
 keywords = ["satellite", "plasma-physics", "nasa-data", "amda", "cdpp", "CDF"]
 authors = [

tests/test_event.py

@@ -206,3 +206,41 @@ def test_remove_events_from_catalogue_unknown_uuid(self):
         e = create_event(t1, t2, "Patrick")
         with self.assertRaises(ValueError):
             tscat.remove_events_from_catalogue("nonexistent-uuid", [e])
+
+    def test_remove_event_fetched_via_predicate_then_member_check(self):
+        # Regression: remove_events_from_catalogue compared by Python id() which
+        # broke whenever the caller looked the event up via get_events(uuid=...)
+        # (fast path → _LazyBackendEntity proxy, different object than the one
+        # already in catalogue.events).
+        t1 = dt.datetime.now()
+        t2 = t1 + dt.timedelta(days=1)
+        e = create_event(t1, t2, "Patrick")
+        c = tscat.create_catalogue("c", "Patrick")
+        tscat.add_events_to_catalogue(c, [e])
+
+        from tscat.filtering import UUID
+        e_again = get_events(UUID(e.uuid))[0]
+        tscat.remove_events_from_catalogue(c, [e_again])
+
+        events, _ = tscat.get_events(c)
+        self.assertEqual(events, [])
+
+    def test_recreate_event_with_same_uuid_after_permanent_delete(self):
+        # Regression: catalogue.events kept the deleted ORM Event, so
+        # add_events_to_catalogue rejected a freshly-created event with the
+        # same UUID even though tscat.get_events(catalogue) reported it empty.
+        t1 = dt.datetime.now()
+        t2 = t1 + dt.timedelta(days=1)
+        e = create_event(t1, t2, "Patrick")
+        c = tscat.create_catalogue("c", "Patrick")
+        tscat.add_events_to_catalogue(c, [e])
+
+        original_uuid = e.uuid
+        e.remove(permanently=True)
+
+        e2 = create_event(t1, t2, "Patrick", uuid=original_uuid)
+        tscat.add_events_to_catalogue(c, [e2])
+
+        events, _ = tscat.get_events(c)
+        self.assertEqual(len(events), 1)
+        self.assertEqual(events[0].uuid, original_uuid)

tscat/__init__.py

@@ -2,7 +2,7 @@
 
 __author__ = """Patrick Boettcher"""
 __email__ = 'p@yai.se'
-__version__ = '0.5.0'
+__version__ = '0.5.1'
 
 from .base import (create_event, create_catalogue,
                    add_events_to_catalogue, remove_events_from_catalogue,

tscat/base.py

@@ -39,6 +39,20 @@ def __setattr__(self, name, value):
             setattr(self._resolve(), name, value)
 
 
+def _resolve_be(be):
+    """Resolve a _LazyBackendEntity proxy to the real ORM entity.
+
+    The fast-path query (get_events_raw) wraps rows in _LazyBackendEntity to
+    defer ORM hydration. Code paths that hand the backend entity to SQLAlchemy
+    relationships (catalogue.events.{extend,remove}, session.delete) need the
+    real, identity-mapped ORM object — passing a proxy stores the proxy in the
+    relationship, breaking later membership checks.
+    """
+    if isinstance(be, _LazyBackendEntity):
+        return be._resolve()
+    return be
+
+
 def backend() -> orm_sqlalchemy.Backend:
     global _backend
     if not _backend:  # pragma: no cover
@@ -91,13 +105,13 @@ def create_catalogue(self, *args: Any, **kwargs: Any) -> '_Catalogue':
 
     @staticmethod
     def add_events_to_catalogue(catalogue: '_Catalogue', events: Union['_Event', List['_Event']]):
-        backend().add_events_to_catalogue(catalogue._backend_entity,
-                                          [event._backend_entity for event in _listify(events)])
+        backend().add_events_to_catalogue(_resolve_be(catalogue._backend_entity),
+                                          [_resolve_be(event._backend_entity) for event in _listify(events)])
 
     @staticmethod
     def remove_events_from_catalogue(catalogue: '_Catalogue', events: Union['_Event', List['_Event']]):
-        backend().remove_events_from_catalogue(catalogue._backend_entity,
-                                               [event._backend_entity for event in _listify(events)])
+        backend().remove_events_from_catalogue(_resolve_be(catalogue._backend_entity),
+                                               [_resolve_be(event._backend_entity) for event in _listify(events)])
 
 
 
@@ -161,7 +175,7 @@ def __eq__(self, o):
     def remove(self, permanently: bool = False) -> None:
         self._removed = True
 
-        backend().remove(self._backend_entity, permanently=permanently)
+        backend().remove(_resolve_be(self._backend_entity), permanently=permanently)
         if permanently:
             del self._backend_entity
 

tscat/orm_sqlalchemy/__init__.py

@@ -229,12 +229,12 @@ def add_events_to_catalogue(self, catalogue: orm.Catalogue, events: List[orm.Eve
         catalogue.events.extend(events)
 
     def remove_events_from_catalogue(self, catalogue: orm.Catalogue, events: List[orm.Event]) -> None:
-        existing = {id(e) for e in catalogue.events}
+        existing_ids = {e.id for e in catalogue.events}
         for e in events:
-            if id(e) not in existing:
+            if e.id not in existing_ids:
                 raise ValueError('Event is not in catalogue.')
-        to_remove = {id(e) for e in events}
-        catalogue.events = [e for e in catalogue.events if id(e) not in to_remove]
+        to_remove_ids = {e.id for e in events}
+        catalogue.events = [e for e in catalogue.events if e.id not in to_remove_ids]
 
     def update_field(self, entity: Union[orm.Event, orm.Catalogue], key: str, value) -> None:
         if key == 'predicate' and value is not None:
@@ -353,6 +353,13 @@ def has_unsaved_changes(self) -> bool:
 
     def remove(self, entity: Union[orm.Catalogue, orm.Event], permanently: bool = False) -> None:
         if permanently:
+            if isinstance(entity, orm.Event):
+                # Detach from any catalogue.events still holding a reference,
+                # otherwise the in-memory collection keeps the deleted ORM
+                # object and later add_events_to_catalogue() rejects a fresh
+                # event reusing the same UUID.
+                for catalogue in list(entity.catalogues):  # type: ignore[attr-defined]
+                    catalogue.events.remove(entity)
             self.session.delete(entity)
             self.session.flush()
         else: