Merge pull request #90 from g-tom/sqlalchemy_cleanup

SQLAlchemy related improvements

ringo/lib/extension.py

@@ -29,10 +29,10 @@
 
 """
 import logging
-import sqlalchemy as sa
 import transaction
 from ringo.lib.sql.db import DBSession
 from ringo.model.modul import ModulItem, ACTIONS
+from ringo.lib.helpers import get_modul_by_name
 
 log = logging.getLogger(name=__name__)
 
@@ -105,22 +105,6 @@ def _add_modul(config, actions, dbsession):
     return modul
 
 
-def _load_modul_by_name(session, modulname):
-    try:
-        # FIXME:
-        # Compatibilty mode. Older versions of Ringo added a 's' to the
-        # extensions modul name as Ringo usally uses the plural form.
-        # Newer versions use the configured extension name. So there
-        # might be a mixture of old and new modul names in the database.
-        # This code will handle this. (ti) <2016-01-04 13:50>
-        return session.query(ModulItem)\
-               .filter(sa.or_(ModulItem.name == modulname,
-                              ModulItem.name == modulname + 's'))\
-                .one()
-    except sa.orm.exc.NoResultFound:
-        return None
-
-
 def register_modul(config, modul_config, actions=None):
     return check_register_modul(config, modul_config, actions)
 
@@ -142,7 +126,7 @@ def check_register_modul(config, modul_config, actions=None):
     """
 
     modulname = modul_config.get('name')
-    modul = _load_modul_by_name(DBSession, modulname)
+    modul = get_modul_by_name(modulname)
     if modul:
         log.info("Extension '%s' OK" % modulname)
     else:

ringo/lib/helpers/__init__.py

@@ -31,6 +31,7 @@
     get_raw_value,
     set_raw_value,
     dynamic_import,
+    get_modul_by_name,
     import_model,
     get_item_modul,
     get_modules,

ringo/lib/helpers/misc.py

@@ -2,6 +2,7 @@
 import re
 import string
 import base64
+import sqlalchemy as sa
 from datetime import datetime
 from pyramid.threadlocal import get_current_request, get_current_registry
 import formbar.converters as converters
@@ -211,6 +212,19 @@ def dynamic_import(cl):
     return getattr(m, classname)
 
 
+def get_modul_by_name(modulname, session=DBSession):
+    # FIXME:
+    # Compatibilty mode. Older versions of Ringo added a 's' to the
+    # extensions modul name as Ringo usally uses the plural form.
+    # Newer versions use the configured extension name. So there
+    # might be a mixture of old and new modul names in the database.
+    # This code will handle this. (ti) <2016-01-04 13:50>
+    from ringo.model.modul import ModulItem
+    return session.query(ModulItem).filter(
+        sa.or_(ModulItem.name == modulname,
+               ModulItem.name == modulname + 's')).scalar()
+
+
 def import_model(clazzpath):
     """Will return the clazz defined by modul entry in the database of
     the given model. The clazzpath defines the base clazz which which
@@ -225,7 +239,7 @@ def import_model(clazzpath):
     orig_clazz = dynamic_import(clazzpath)
     # Load entry from the database for the given modul
     mid = orig_clazz._modul_id
-    modul = DBSession.query(ModulItem).filter_by(id=mid).one()
+    modul = DBSession.query(ModulItem).get(mid)
     if modul.clazzpath == clazzpath:
         return orig_clazz
     else:

ringo/lib/imexport.py

@@ -454,8 +454,6 @@ def perform(self, data, user=None, translate=lambda x: x, load_key="uuid"):
                 else:
                     id = values.get(load_key)
                 try:
-                    # uuid might be empty for new items, which will raise an
-                    # error on loading.
                     item = factory.load(id, field=load_key)
                     item.set_values(values, use_strict=self._use_strict)
                     operation = _("UPDATE")

ringo/lib/security.py

@@ -128,10 +128,7 @@ def load_user(login):
     :returns: User or None
 
     """
-    try:
-        return DBSession.query(User).filter_by(login=login).one()
-    except NoResultFound:
-        return None
+    return DBSession.query(User).filter_by(login=login).scalar()
 
 
 def csrf_token_validation(event):

ringo/model/base.py

@@ -17,6 +17,7 @@
 import Levenshtein
 from sqlalchemy import Column, CHAR
 from sqlalchemy.orm import joinedload, Session
+from sqlalchemy.orm.exc import NoResultFound
 from ringo.lib.helpers import (
     serialize, get_item_modul,
     get_raw_value, set_raw_value,
@@ -115,7 +116,7 @@ def load_modul(item):
     # Loading the modul is expensive! So try to cache it.
     if not CACHE_MODULES.get(mid):
         if session:
-            modul = session.query(ModulItem).filter_by(id=mid).one()
+            modul = session.query(ModulItem).get(mid)
         else:
             modul = get_item_modul(None, item)
         CACHE_MODULES.set(modul.id, modul)
@@ -865,7 +866,7 @@ def __init__(self, clazz, request=None, use_strict=False):
         self._use_strict = use_strict
 
     def create(self, user, values):
-        """Will create a new instance of clazz. The instance is it is
+        """Will create a new instance of clazz. The instance is
         not saved persistent at this moment. The method will also take
         care of setting the correct ownership.
 
@@ -883,7 +884,7 @@ def create(self, user, values):
            user is not None):
             item.uid = user.id
         if (hasattr(item, 'gid')):
-            modul = get_item_modul(None, item)
+            modul = load_modul(item)
             if modul.default_gid:
                 item.gid = modul.default_gid
             elif (user is not None and user.default_gid):
@@ -895,21 +896,20 @@ def create(self, user, values):
                 item.set_values(values)
         return item
 
-    def load(self, id, db=None, cache="", uuid=False, field=None):
+    def load(self, id, db=DBSession, cache="", uuid=False, field=None):
         """Loads the item with id from the database and returns it.
 
-        :id: ID of the item to be loaded
+        :id: Primary key or field value (if field is given) of the item to
+             be loaded
         :db: DB session to load the item
         :cache: Name of the cache region. If empty then no caching is
                 done.
-        :uuid: If true the given id is a uuid. Default to false
-        :field: If given the item can be loaded by an alternative field.
-                Default to None
+        :uuid: (deprecated) If True the given id is a uuid. Defaults to False
+        :field: If given, id is expected to be a unique value of the given
+                field. Defaults to None
         :returns: Instance of clazz
 
         """
-        if db is None:
-            db = DBSession
         q = db.query(self._clazz)
         if cache in regions.keys():
             q = set_relation_caching(q, self._clazz, cache)
@@ -924,4 +924,8 @@ def load(self, id, db=None, cache="", uuid=False, field=None):
                           "field='uuid' instead", DeprecationWarning)
             return q.filter(self._clazz.uuid == id).one()
         else:
-            return q.filter(self._clazz.id == id).one()
+            item = q.get(id)
+            if item:
+                return item
+            else:
+                raise NoResultFound

ringo/scripts/application.py

@@ -1,8 +1,8 @@
 import os
 import transaction
 from ringo.scripts.db import get_session
-from ringo.lib.helpers import get_app_location, dynamic_import
-from ringo.lib.extension import _add_modul, _load_modul_by_name
+from ringo.lib.helpers import get_app_location, dynamic_import, get_modul_by_name
+from ringo.lib.extension import _add_modul
 from ringo.model import extensions
 
 
@@ -49,7 +49,7 @@ def handle_ext_init_command(args):
     session = get_session(os.path.join(*path))
     for ext in extensions:
         if ext == args.name:
-            if _load_modul_by_name(session, ext.replace("ringo_", "")):
+            if get_modul_by_name(ext.replace("ringo_", ""), session):
                 print "Extension %s already added!" % args.name
             else:
                 extension = dynamic_import("%s." % args.name)
@@ -65,7 +65,7 @@ def handle_ext_delete_command(args):
     path.append(get_app_location(args.app))
     path.append(args.config)
     session = get_session(os.path.join(*path))
-    modul = _load_modul_by_name(session, args.name.replace("ringo_", ""))
+    modul = get_modul_by_name(args.name.replace("ringo_", ""), session)
     if modul:
         if args.name in extensions:
             session.delete(modul)

ringo/scripts/db.py

@@ -5,7 +5,6 @@
 import time
 import json
 from sqlalchemy import engine_from_config
-from sqlalchemy.orm.exc import NoResultFound
 import transaction
 
 from invoke import run
@@ -17,7 +16,7 @@
     setup_logging,
 )
 from ringo.lib.sql import DBSession, NTDBSession, setup_db_session
-from ringo.lib.helpers import get_app_location, dynamic_import
+from ringo.lib.helpers import get_app_location, dynamic_import, get_modul_by_name
 from ringo.lib.imexport import (
     JSONExporter, JSONImporter,
     CSVExporter, CSVImporter,
@@ -98,6 +97,17 @@ def get_session(config_file, transactional=True):
         return NTDBSession
 
 
+def get_modul(modulname, session):
+    modul_item = get_modul_by_name(modulname, session)
+    if not modul_item:
+        modules = [m.name for m in session.query(ModulItem).all()]
+        msg = "Can not load modul '{}'. Please choose one from [{}].".format(
+            modulname, ", ".join(modules))
+        print >> sys.stderr, msg
+        sys.exit(1)
+    return dynamic_import(modul_item.clazzpath)
+
+
 def copy_initial_migration_scripts(args):
     """Will copy the initial db migration scripts into the alembic
     versions folder of the application.
@@ -189,8 +199,7 @@ def handle_db_savedata_command(args):
     path = []
     path.append(args.config)
     session = get_session(os.path.join(*path))
-    modul_clazzpath = session.query(ModulItem).filter(ModulItem.name == args.modul).all()[0].clazzpath
-    modul = dynamic_import(modul_clazzpath)
+    modul = get_modul(args.modul, session)
     data = session.query(modul).order_by(modul.id).all()
 
     if args.filter:
@@ -256,19 +265,11 @@ def handle_db_loaddata_command(args):
 
 
 def get_importer(session, modulname, fmt):
-    try:
-        modul_clazzpath = session.query(ModulItem).filter(
-            ModulItem.name == modulname).one().clazzpath
-        modul = dynamic_import(modul_clazzpath)
-        if fmt == "json":
-            return JSONImporter(modul, session)
-        else:
-            return CSVImporter(modul, session)
-    except NoResultFound:
-        modules = [m.name for m in session.query(ModulItem).all()]
-        print "Can not load modul '{}'. Please choose one from [{}].".format(
-            modulname, ", ".join(modules))
-        sys.exit(1)
+    modul = get_modul(modulname, session)
+    if fmt == "json":
+        return JSONImporter(modul, session)
+    else:
+        return CSVImporter(modul, session)
 
 
 def do_import(session, importer, data, load_key):
@@ -279,7 +280,6 @@ def do_import(session, importer, data, load_key):
     for item, action in items:
         # Add all new items to the session
         if action.find("CREATE") > -1:
-            session.add(item)
             created += 1
         else:
             updated += 1
@@ -290,8 +290,7 @@ def handle_db_uuid_command(args):
     path = []
     path.append(args.config)
     session = get_session(os.path.join(*path))
-    modul_clazzpath = session.query(ModulItem).filter(ModulItem.name == args.modul).all()[0].clazzpath
-    modul = dynamic_import(modul_clazzpath)
+    modul = get_modul(args.modul, session)
     updated = 0
     for item in session.query(modul).all():
         item.reset_uuid()

ringo/views/users.py

@@ -219,7 +219,9 @@ def setstandin(request, allowed_users=None):
         if request.GET.get('backurl'):
             user_id = urlparse.urlparse(
                 request.GET.get('backurl')).path.split('/')[-1]
-            user = request.db.query(User).filter(User.id == user_id).one()
+            user = request.db.query(User).get(user_id)
+            if not user:
+                raise HTTPBadRequest()
         # Otherwise the user sets the standin of his own group. In this
         # case the user is already in the request.
         else:
@@ -330,7 +332,6 @@ def removeaccount(request):
     _ = request.translate
     # Load the item return 400 if the item can not be found.
     factory = clazz.get_item_factory()
-
     try:
         item = factory.load(id, request.db)
         # Check authorisation