Merge pull request #172 from jeremycline/scoped-session

Start creating a scoped session object in the models module

fmn/lib/db.py

@@ -20,50 +20,68 @@
 """
 from __future__ import print_function, unicode_literals
 
-import sys
+import argparse
+
+from fmn.lib import models
 
-import fedmsg.config
-import fmn.lib.models
 
 def main():
     """
     The entry point for the database commands.
     """
-    config = fedmsg.config.load_config()
-    uri = config.get('fmn.sqlalchemy.uri')
-    if not uri:
-        raise ValueError("fmn.sqlalchemy.uri must be present")
+    parser = argparse.ArgumentParser(description='FMN database manager')
+    parser.add_argument(
+        '--create',
+        '-c',
+        dest='create',
+        action='store_true',
+        help='Create the database tables'
+    )
+    parser.add_argument(
+        '--with-dev-data',
+        '-d',
+        dest='dev',
+        action='store_true',
+        help='Add some development sample data'
+    )
+    args = parser.parse_args()
+
+    if args.create:
+        models.BASE.metadata.create_all(models.engine)
 
-    if '-h' in sys.argv or '--help'in sys.argv:
-        print(sys.argv[0] + " [--with-dev-data]")
-        sys.exit(0)
+    if args.dev:
+        dev_data()
 
-    session = fmn.lib.models.init(uri, debug=True, create=True)
 
-    if '--with-dev-data' in sys.argv:
-        context1 = fmn.lib.models.Context.create(
-            session, name="irc", description="Internet Relay Chat",
-            detail_name="irc nick", icon="user",
-            placeholder="z3r0_c00l",
-        )
-        context2 = fmn.lib.models.Context.create(
-            session, name="email", description="Electronic Mail",
-            detail_name="email address", icon="envelope",
-            placeholder="jane@fedoraproject.org",
-        )
-        context3 = fmn.lib.models.Context.create(
-            session, name="android", description="Google Cloud Messaging",
-            detail_name="registration id", icon="phone",
-            placeholder="laksdjfasdlfkj183097falkfj109f"
-        )
-        context4 = fmn.lib.models.Context.create(
-            session, name="desktop", description="fedmsg-notify",
-            detail_name="None", icon="console",
-            placeholder="There's no need to put a value here"
-        )
-        context5 = fmn.lib.models.Context.create(
-            session, name="sse", description="server sent events",
-            detail_name="None", icon="console",
-            placeholder="There's no need to put a value here"
-        )
-        session.commit()
+def dev_data():
+    """
+    Populate the database with some development data
+    """
+    session = models.Session()
+    models.Context.create(
+        session, name="irc", description="Internet Relay Chat",
+        detail_name="irc nick", icon="user",
+        placeholder="z3r0_c00l",
+    )
+    models.Context.create(
+        session, name="email", description="Electronic Mail",
+        detail_name="email address", icon="envelope",
+        placeholder="jane@fedoraproject.org",
+    )
+    models.Context.create(
+        session, name="android", description="Google Cloud Messaging",
+        detail_name="registration id", icon="phone",
+        placeholder="laksdjfasdlfkj183097falkfj109f"
+    )
+    models.Context.create(
+        session, name="desktop", description="fedmsg-notify",
+        detail_name="None", icon="console",
+        placeholder="There's no need to put a value here"
+    )
+    models.Context.create(
+        session, name="sse", description="server sent events",
+        detail_name="None", icon="console",
+        placeholder="There's no need to put a value here"
+    )
+    session.commit()
+    models.Session.remove()

fmn/lib/models.py

@@ -36,16 +36,27 @@
 from sqlalchemy import create_engine
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
-from sqlalchemy.orm import sessionmaker
-from sqlalchemy.orm import scoped_session
-from sqlalchemy.orm import relation
-from sqlalchemy.orm import backref
+from sqlalchemy.orm import sessionmaker, scoped_session, backref, relation
 
 import fedmsg
 import fedmsg.utils
 
 import fmn.lib.defaults
 
+_config = fedmsg.config.load_config()
+
+#: The SQLAlchemy database engine, initialized with the URL in the fedmsg config key
+#: ``fmn.sqlalchemy.uri`` and ``fmn.sqlalchemy.debug`` (bool). If the debug setting is
+#: true, SQLAlchemy will log all the raw SQL statements it generates.
+engine = create_engine(
+    _config.get('fmn.sqlalchemy.uri'), echo=_config.get('fmn.sqlalchemy.debug', False))
+
+#: An SQLAlchemy scoped session. This session can be optionally called to return
+#: the thread-local session or used directly (in which case it creates or uses the
+#: existing thread-local session). Call ``Session.remove()`` to remove the session
+#: once you are done with it. A new one will be created on next use.
+Session = scoped_session(sessionmaker(bind=engine))
+
 
 class FMNBase(object):
     def notify(self, openid, context, changed):
@@ -70,28 +81,25 @@ def init(db_url, alembic_ini=None, debug=False, create=False):
     """ Create the tables in the database using the information from the
     url obtained.
 
-    :arg db_url, URL used to connect to the database. The URL contains
-        information with regards to the database engine, the host to
-        connect to, the user and password and the database name.
-          ie: <engine>://<user>:<password>@<host>/<dbname>
-    :kwarg alembic_ini, path to the alembic ini file. This is necessary
-        to be able to use alembic correctly, but not for the unit-tests.
-    :kwarg debug, a boolean specifying wether we should have the verbose
-        output of sqlalchemy or not.
-    :return a session that can be used to query the database.
-
+    .. deprecated:: 1.2.0
+       Use the session created in this module and the fmn-createdb script instead
+
+    Args:
+        db_url (str): URL used to connect to the database. The URL contains
+            information with regards to the database engine, the host to
+            connect to, the user and password and the database name.
+            ie: <engine>://<user>:<password>@<host>/<dbname>
+        alembic_ini (str): path to the alembic ini file. This is necessary
+            to be able to use alembic correctly, but not for the unit-tests.
+        debug (bool): a boolean specifying wether we should have the verbose
+            output of sqlalchemy or not.
+
+    Returns:
+        scopedsession: An SQLAlchemy scoped session.
     """
-    engine = create_engine(db_url, echo=debug)
-
     if create:
         BASE.metadata.create_all(engine)
 
-    # This... "causes problems"
-    #if db_url.startswith('sqlite:'):
-    #    def _fk_pragma_on_connect(dbapi_con, con_record):
-    #        dbapi_con.execute('pragma foreign_keys=ON')
-    #    sa.event.listen(engine, 'connect', _fk_pragma_on_connect)
-
     if alembic_ini is not None:  # pragma: no cover
         # then, load the Alembic configuration and generate the
         # version table, "stamping" it with the most recent rev:
@@ -100,8 +108,9 @@ def init(db_url, alembic_ini=None, debug=False, create=False):
         alembic_cfg = Config(alembic_ini)
         command.stamp(alembic_cfg, "head")
 
-    scopedsession = scoped_session(sessionmaker(bind=engine))
-    return scopedsession
+    # Return the scoped session created in the db module for code still using
+    # this funciton
+    return Session
 
 
 class Context(BASE):

fmn/tests/__init__.py

@@ -1,7 +1,11 @@
+import unittest
 import os
+
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker, scoped_session
+
 import fmn.lib.models
 
-import unittest
 
 DB_PATH = 'sqlite:////var/tmp/test-fmn-lib.sqlite'
 
@@ -11,7 +15,13 @@ def setUp(self):
         dbfile = DB_PATH.split('///')[1]
         if os.path.exists(dbfile):
             os.unlink(dbfile)
-        self.sess = fmn.lib.models.init(DB_PATH, debug=False, create=True)
+
+        self._old_engine = fmn.lib.models.engine
+        self._old_session = fmn.lib.models.Session
+        fmn.lib.models.engine = create_engine(DB_PATH, echo=False)
+        fmn.lib.models.Session = scoped_session(sessionmaker(bind=fmn.lib.models.engine))
+        fmn.lib.models.BASE.metadata.create_all(fmn.lib.models.engine)
+        self.sess = fmn.lib.models.Session
 
         self.config = {
             'fmn.backends': ['irc', 'email', 'android'],
@@ -33,6 +43,10 @@ def tearDown(self):
         if os.path.exists(dbfile):
             os.unlink(dbfile)
 
-        self.sess.rollback()
+        # Remove the session from the session registry and roll back any
+        # transaction state.
+        fmn.lib.models.Session.remove()
 
         fmn.lib.models.FMNBase.notify = self.original_notify
+        fmn.lib.models.engine = self._old_engine
+        fmn.lib.models.Session = self._old_session

fmn/tests/lib/test_db.py

@@ -0,0 +1,65 @@
+# -*- coding: utf-8 -*-
+#
+# This file is part of the FMN project.
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+"""Unit tests for the :mod:`fmn.lib.db` module."""
+
+import unittest
+
+import mock
+
+from fmn.lib import db, models
+from fmn.tests import Base as BaseTestCase
+
+
+class MainTests(BaseTestCase):
+    """Tests for the DB CLI entry point"""
+
+    @mock.patch('sys.argv', ['fmn-createdb'])
+    @mock.patch('fmn.lib.db.dev_data')
+    @mock.patch('fmn.lib.db.models.BASE.metadata.create_all')
+    def test_main_no_create_no_dev_data(self, mock_create, mock_dev_data):
+        """Assert nothing happens without some arguments"""
+        db.main()
+        self.assertEqual(0, mock_create.call_count)
+        self.assertEqual(0, mock_dev_data.call_count)
+
+    @mock.patch('sys.argv', 'fmn-createdb --with-dev-data'.split())
+    @mock.patch('fmn.lib.db.models.BASE.metadata.create_all')
+    def test_main_no_create_dev_data(self, mock_create):
+        """Assert --with-dev-data adds data"""
+        contexts = models.Session.query(models.Context).all()
+        self.assertEqual(0, len(contexts))
+        db.main()
+        self.assertEqual(0, mock_create.call_count)
+        contexts = models.Session.query(models.Context).all()
+        self.assertEqual(5, len(contexts))
+
+    @mock.patch('sys.argv', 'fmn-createdb --create --with-dev-data'.split())
+    @mock.patch('fmn.lib.db.models.BASE.metadata.create_all')
+    def test_main_create_dev_data(self, mock_create):
+        """Assert -c -d creates a db and adds data"""
+        contexts = models.Session.query(models.Context).all()
+        self.assertEqual(0, len(contexts))
+        db.main()
+        mock_create.assert_called_once_with(models.engine)
+        contexts = models.Session.query(models.Context).all()
+        self.assertEqual(5, len(contexts))
+
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)

fmn/tests/test_models.py → fmn/tests/lib/test_models.py

@@ -1,7 +1,50 @@
+# -*- coding: utf-8 -*-
+#
+# This file is part of the FMN project.
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+"""Unit tests for the :mod:`fmn.lib.models` module."""
+
+import unittest
+
+import mock
+
 import fmn.lib.models
 import fmn.tests
 
 
+class TestFMNBase(unittest.TestCase):
+
+    def test_scoped_session(self):
+        """Assert the module creates a scoped session"""
+        session1 = fmn.lib.models.Session()
+        session2 = fmn.lib.models.Session()
+
+        self.assertTrue(session1 is session2)
+
+    @mock.patch('fmn.lib.models.fedmsg.publish')
+    def test_base_has_notify(self, mock_publish):
+        """Assert the model base class has a notify method"""
+        fmn.lib.models.BASE.notify(fmn.lib.models.BASE(), 'jcline', 'email', 'change')
+        mock_publish.assert_called_once_with(
+            topic='base.update',
+            msg={'openid': 'jcline', 'context': 'email', 'changed': 'change'},
+        )
+
+
 class TestBasics(fmn.tests.Base):
     def test_setup_and_teardown(self):
         pass