From 0a675430c1b5806b4627f39ed3ccce9d5fbaa669 Mon Sep 17 00:00:00 2001
From: Lars Kruse <devel@sumpfralle.de>
Date: Sat, 22 Jan 2022 16:31:29 +0100
Subject: [PATCH] Revert "refactor: use short-lived sessions when accessing the
 database"

The introduction of short-lived sessions caused issues with functions,
which return database objects.  Such objects are not accessible (due to
lazy loading), after the session is closed.

Instead we should either move the session scope the respective callers
or use a request-based session handling (e.g. the one provided by
flask-sqlalchemy).

This reverts commit 0b594e5021 and 70ffd1a05c.

See: #93
---
 src/scheduling/fallback.py |   3 +-
 src/scheduling/models.py   | 162 +++++++++++--------------------------
 2 files changed, 47 insertions(+), 118 deletions(-)

diff --git a/src/scheduling/fallback.py b/src/scheduling/fallback.py
index bcf669f4..6a6133aa 100644
--- a/src/scheduling/fallback.py
+++ b/src/scheduling/fallback.py
@@ -138,8 +138,7 @@ class FallbackManager:
             if fallback_type != self.state.get("previous_fallback_type"):
                 timeslot = self.state["timeslot"]
                 if timeslot:
-                    with DB.Session() as session:
-                        session.merge(timeslot)
+                    DB.session.merge(timeslot)
                 self.engine.event_dispatcher.on_fallback_active(timeslot, fallback_type)
 
 
diff --git a/src/scheduling/models.py b/src/scheduling/models.py
index b014727c..be82741d 100644
--- a/src/scheduling/models.py
+++ b/src/scheduling/models.py
@@ -17,7 +17,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-import contextlib
 import sys
 import time
 import logging
@@ -25,7 +24,6 @@ import datetime
 
 import sqlalchemy as sa
 
-import sqlalchemy
 from sqlalchemy import BigInteger, Boolean, Column, DateTime, Integer, String, ForeignKey, ColumnDefault
 from sqlalchemy.orm import scoped_session
 from sqlalchemy.orm import sessionmaker
@@ -42,40 +40,14 @@ config = AuraConfig()
 engine = sa.create_engine(config.get_database_uri())
 Base = declarative_base()
 Base.metadata.bind = engine
-__sqlalchemy_version = tuple(int(item) for item in sqlalchemy.__version__.split(".")[:2])
-
 
 class DB():
     session_factory = sessionmaker(bind=engine)
     Session = scoped_session(session_factory)
+    session = Session()
     Model = Base
 
 
-# Monkey-patch the above DB.Session generator for SQLAlchemy before v1.4.
-# Such older versions of SQLAlchemy do not support contexts.
-if __sqlalchemy_version < (1, 4):
-    @contextlib.contextmanager
-    def get_session_context():
-        """ provide a context for a session
-
-        This context is the same as the one provided by a "scoped_session" in SQLAlchemy v1.4 or
-        later.
-
-        see https://docs.sqlalchemy.org/en/13/orm/session_basics.html#when-do-i-construct-a-session-when-do-i-commit-it-and-when-do-i-close-it
-        """
-        session = scoped_session(DB.session_factory)
-
-        # Commented out because of https://gitlab.servus.at/aura/engine/-/issues/90
-        # try:
-        #     yield session
-        # finally:
-        #     session.close()
-
-        yield session
-
-
-    DB.Session = get_session_context
-
 
 class AuraDatabaseModel():
     """
@@ -97,32 +69,29 @@ class AuraDatabaseModel():
         """
         Store to the database
         """
-        with DB.Session() as session:
-            if add:
-                session.add(self)
-            else:
-                session.merge(self)
-            if commit:
-                session.commit()
+        if add:
+            DB.session.add(self)
+        else:
+            DB.session.merge(self)
+        if commit:
+            DB.session.commit()
 
 
     def delete(self, commit=False):
         """
         Delete from the database
         """
-        with DB.Session() as session:
-            session.delete(self)
-            if commit:
-                session.commit()
+        DB.session.delete(self)
+        if commit:
+            DB.session.commit()
 
 
     def refresh(self):
         """
         Refreshes the correct record
         """
-        with DB.Session() as session:
-            session.expire(self)
-            session.refresh(self)
+        DB.session.expire(self)
+        DB.session.refresh(self)
 
 
     def _asdict(self):
@@ -163,8 +132,7 @@ class AuraDatabaseModel():
         """
         Base.metadata.drop_all()
         Base.metadata.create_all()
-        with DB.Session() as session:
-            session.commit()
+        DB.session.commit()
 
         if systemexit:
             sys.exit(0)
@@ -197,27 +165,27 @@ class Timeslot(DB.Model, AuraDatabaseModel):
     playlist = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.playlist_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
     default_schedule_playlist = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.default_schedule_playlist_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
     default_show_playlist = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.default_show_playlist_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
     schedule_fallback = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.schedule_fallback_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
     show_fallback = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.show_fallback_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
     station_fallback = relationship("Playlist",
                             primaryjoin="and_(Timeslot.timeslot_start==Playlist.timeslot_start, \
                                 Timeslot.station_fallback_id==Playlist.playlist_id, Timeslot.show_name==Playlist.show_name)",
-                            uselist=False, back_populates="timeslot", lazy='subquery')
+                            uselist=False, back_populates="timeslot")
 
     playlist_id = Column(Integer)
     default_schedule_playlist_id = Column(Integer)
@@ -255,12 +223,7 @@ class Timeslot(DB.Model, AuraDatabaseModel):
         Args:
             date_time (datetime): date and time when the timeslot starts
         """
-        with DB.Session() as session:
-            return (
-                session.query(Timeslot)
-                .filter(Timeslot.timeslot_start == date_time)
-                .first()
-            )
+        return DB.session.query(Timeslot).filter(Timeslot.timeslot_start == date_time).first()
 
 
     @staticmethod
@@ -275,13 +238,10 @@ class Timeslot(DB.Model, AuraDatabaseModel):
         Returns:
             ([Timeslot]):           List of timeslots
         """
-        with DB.Session() as session:
-            return (
-                session.query(Timeslot)
-                .filter(Timeslot.timeslot_start >= date_from)
-                .order_by(Timeslot.timeslot_start)
-                .all()
-            )
+        timeslots = DB.session.query(Timeslot).\
+            filter(Timeslot.timeslot_start >= date_from).\
+            order_by(Timeslot.timeslot_start).all()
+        return timeslots
 
 
     def set_active_entry(self, entry):
@@ -381,8 +341,8 @@ class Playlist(DB.Model, AuraDatabaseModel):
     timeslot_start = Column(DateTime, ForeignKey("timeslot.timeslot_start"))
 
     # Relationships
-    timeslot = relationship("Timeslot", uselist=False, back_populates="playlist", lazy='subquery')
-    entries = relationship("PlaylistEntry", back_populates="playlist", lazy='subquery')
+    timeslot = relationship("Timeslot", uselist=False, back_populates="playlist")
+    entries = relationship("PlaylistEntry", back_populates="playlist")
 
     # Data
     playlist_id = Column(Integer, autoincrement=False)
@@ -395,8 +355,7 @@ class Playlist(DB.Model, AuraDatabaseModel):
     #     """
     #     Fetches all entries
     #     """
-    #     with DB.Session() as session:
-    #         all_entries = session.query(Playlist).filter(Playlist.fallback_type == 0).all()
+    #     all_entries = DB.session.query(Playlist).filter(Playlist.fallback_type == 0).all()
 
     #     cnt = 0
     #     for entry in all_entries:
@@ -422,12 +381,7 @@ class Playlist(DB.Model, AuraDatabaseModel):
             Exception:              In case there a inconsistent database state, such es multiple playlists for given date/time.
         """
         playlist = None
-        with DB.Session() as session:
-            playlists = (
-                session.query(Playlist)
-                .filter(Playlist.timeslot_start == start_date)
-                .all()
-            )
+        playlists = DB.session.query(Playlist).filter(Playlist.timeslot_start == start_date).all()
 
         for p in playlists:
             if p.playlist_id == playlist_id:
@@ -447,13 +401,7 @@ class Playlist(DB.Model, AuraDatabaseModel):
         Returns:
             (Array<Playlist>):      An array holding the playlists
         """
-        with DB.Session() as session:
-            return (
-                session.query(Playlist)
-                .filter(Playlist.playlist_id == playlist_id)
-                .order_by(Playlist.timeslot_start)
-                .all()
-            )
+        return DB.session.query(Playlist).filter(Playlist.playlist_id == playlist_id).order_by(Playlist.timeslot_start).all()
 
 
     @staticmethod
@@ -461,11 +409,10 @@ class Playlist(DB.Model, AuraDatabaseModel):
         """
         Checks if the given is empty
         """
-        with DB.Session() as session:
-            try:
-                return not session.query(Playlist).one_or_none()
-            except sa.orm.exc.MultipleResultsFound:
-                return False
+        try:
+            return not DB.session.query(Playlist).one_or_none()
+        except sa.orm.exc.MultipleResultsFound:
+            return False
 
 
     @hybrid_property
@@ -540,8 +487,8 @@ class PlaylistEntry(DB.Model, AuraDatabaseModel):
     artificial_playlist_id = Column(Integer, ForeignKey("playlist.artificial_id"))
 
     # Relationships
-    playlist = relationship("Playlist", uselist=False, back_populates="entries", lazy='subquery')
-    meta_data = relationship("PlaylistEntryMetaData", uselist=False, back_populates="entry", lazy='subquery')
+    playlist = relationship("Playlist", uselist=False, back_populates="entries")
+    meta_data = relationship("PlaylistEntryMetaData", uselist=False, back_populates="entry")
 
     # Data
     entry_num = Column(Integer)
@@ -562,37 +509,26 @@ class PlaylistEntry(DB.Model, AuraDatabaseModel):
         """
         Selects one entry identified by `playlist_id` and `entry_num`.
         """
-        with DB.Session() as session:
-            return (
-                session.query(PlaylistEntry)
-                .filter(PlaylistEntry.entry_num == entry_num)
-                .filter(PlaylistEntry.artificial_playlist_id == artificial_playlist_id)
-                .first()
-            )
+        return DB.session.query(PlaylistEntry).filter(PlaylistEntry.artificial_playlist_id == artificial_playlist_id, PlaylistEntry.entry_num == entry_num).first()
 
     @staticmethod
     def delete_entry(artificial_playlist_id, entry_num):
         """
         Deletes the playlist entry and associated metadata.
         """
-        with DB.Session() as session:
-            entry = PlaylistEntry.select_playlistentry_for_playlist(artificial_playlist_id, entry_num)
-            metadata = PlaylistEntryMetaData.select_metadata_for_entry(entry.artificial_id)
-            metadata.delete()
-            entry.delete()
-            session.commit()
+        entry = PlaylistEntry.select_playlistentry_for_playlist(artificial_playlist_id, entry_num)
+        metadata = PlaylistEntryMetaData.select_metadata_for_entry(entry.artificial_id)
+        metadata.delete()
+        entry.delete()
+        DB.session.commit()
 
     @staticmethod
     def count_entries(artificial_playlist_id):
         """
         Returns the count of all entries.
         """
-        with DB.Session() as session:
-            return (
-                session.query(PlaylistEntry)
-                .filter(PlaylistEntry.artificial_playlist_id == artificial_playlist_id)
-                .count()
-            )
+        result = DB.session.query(PlaylistEntry).filter(PlaylistEntry.artificial_playlist_id == artificial_playlist_id).count()
+        return result
 
     @hybrid_property
     def entry_end(self):
@@ -688,7 +624,7 @@ class PlaylistEntryMetaData(DB.Model, AuraDatabaseModel):
     artificial_entry_id = Column(Integer, ForeignKey("playlist_entry.artificial_id"))
 
     # Relationships
-    entry = relationship("PlaylistEntry", uselist=False, back_populates="meta_data", lazy='subquery')
+    entry = relationship("PlaylistEntry", uselist=False, back_populates="meta_data")
 
     # Data
     artist = Column(String(256))
@@ -697,13 +633,7 @@ class PlaylistEntryMetaData(DB.Model, AuraDatabaseModel):
 
     @staticmethod
     def select_metadata_for_entry(artificial_playlistentry_id):
-        with DB.Session() as session:
-            return (
-                session.query(PlaylistEntryMetaData)
-                .filter(PlaylistEntryMetaData.artificial_entry_id == artificial_playlistentry_id)
-                .first()
-            )
-
+        return DB.session.query(PlaylistEntryMetaData).filter(PlaylistEntryMetaData.artificial_entry_id == artificial_playlistentry_id).first()
 
 
-Base.metadata.create_all(engine)
\ No newline at end of file
+Base.metadata.create_all(engine)
-- 
GitLab