Renamed some entities to more appropriate names

CHANGES.rst

@@ -1,9 +1,21 @@
 SimPhoNy Remote CHANGELOG
 =========================
 
-What's new in SimPhoNy Remote 1.2.0
+What's new in SimPhoNy Remote 2.0.0
 -----------------------------------
 
+- Renamed some entities to more appropriate names (#419)
+  - The term Accounting now refers exclusively to the relationship table
+    connecting users, applications and policies.
+  - The config file accounting_class and accounting_kwargs are now
+    database_class and database_kwargs.
+  - AppAccounting is now ORMDatabase. Similarly, CSVAccounting is now
+    CSVDatabase.
+  - The above two break compatibility with the old configuration files.
+    Old configuration files must be changed both in the options
+    and the class they refer to.
+
+
 What's new in SimPhoNy Remote 1.1.0
 -----------------------------------
 

doc/source/configuration.rst

@@ -80,6 +80,6 @@ docker setup.
    For example, to use CSV as the database, `/path/to/config.py` would
    contain the followings::
 
-     accounting_class = 'remoteappmanager.db.csv_db.CSVAccounting'
-     accounting_kwargs = {'url': '/path/to/csv_file'}
+     database_class = 'remoteappmanager.db.csv_db.CSVDatabase'
+     database_kwargs = {'url': '/path/to/csv_file'}
 

jupyterhub/remoteappmanager_config.py

@@ -54,14 +54,14 @@
 #
 # # CSV database support
 #
-# accounting_class = "remoteappmanager.db.csv_db.CSVAccounting"
-# accounting_kwargs = {
+# database_class = "remoteappmanager.db.csv_db.CSVDatabase"
+# database_kwargs = {
 #     "csv_file_path": os.path.abspath("./remoteappmanager.csv")}
 #
 # # Sqlite database support
 #
-# accounting_class = "remoteappmanager.db.orm.AppAccounting"
-# accounting_kwargs = {
+# database_class = "remoteappmanager.db.orm.ORMDatabase"
+# database_kwargs = {
 #     "url": "sqlite:///"+os.path.abspath('./remoteappmanager.db')}
 
 # # ----------------

remoteappmanager/base_application.py

@@ -8,7 +8,7 @@
 
 from tornadowebapi.registry import Registry
 
-from remoteappmanager.db.interfaces import ABCAccounting
+from remoteappmanager.db.interfaces import ABCDatabase
 from remoteappmanager.logging.logging_mixin import LoggingMixin
 from remoteappmanager.docker.container_manager import ContainerManager
 from remoteappmanager.jinja2_adapters import Jinja2LoaderAdapter, gravatar_id
@@ -33,7 +33,7 @@ class BaseApplication(web.Application, LoggingMixin):
 
     #: An accounting system that knows the allowed users, what applications
     #: they can run etc.
-    db = Instance(ABCAccounting, allow_none=True)
+    db = Instance(ABCDatabase, allow_none=True)
 
     #: API access to the configurable-http-proxy
     reverse_proxy = Instance(ReverseProxy)
@@ -119,11 +119,11 @@ def _hub_default(self):
     @default("db")
     def _db_default(self):
         """Initializes the database connection."""
-        class_path = self.file_config.accounting_class
+        class_path = self.file_config.database_class
         module_path, _, cls_name = class_path.rpartition('.')
         cls = getattr(importlib.import_module(module_path), cls_name)
         try:
-            return cls(**self.file_config.accounting_kwargs)
+            return cls(**self.file_config.database_kwargs)
         except Exception:
             reason = 'The database is not initialised properly.'
             self.log.exception(reason)

remoteappmanager/cli/remoteappdb/__main__.py

@@ -233,14 +233,14 @@ def list(ctx, no_decoration, show_apps):
             cur = [user.id, user.name]
             table.append(cur)
             if show_apps:
-                apps = [[app.image,
-                         policy.allow_home,
-                         policy.allow_view,
-                         policy.allow_common,
-                         policy.volume_source,
-                         policy.volume_target,
-                         policy.volume_mode]
-                        for _, app, policy in orm.apps_for_user(session, user)]
+                apps = [[entry.application.image,
+                         entry.application_policy.allow_home,
+                         entry.application_policy.allow_view,
+                         entry.application_policy.allow_common,
+                         entry.application_policy.volume_source,
+                         entry.application_policy.volume_target,
+                         entry.application_policy.volume_mode]
+                        for entry in orm.accounting_for_user(session, user)]
 
                 if len(apps) == 0:
                     apps = [['']*7]

remoteappmanager/db/csv_db.py

@@ -47,7 +47,7 @@
 import uuid
 
 from remoteappmanager.db.interfaces import (
-    ABCAccounting, ABCApplication, ABCApplicationPolicy)
+    ABCDatabase, ABCApplication, ABCApplicationPolicy, ABCAccounting)
 from remoteappmanager.db.exceptions import UnsupportedOperation
 from remoteappmanager.utils import mergedocs, one
 
@@ -60,6 +60,10 @@ class CSVApplicationPolicy(ABCApplicationPolicy):
     pass
 
 
+class CSVAccounting(ABCAccounting):
+    pass
+
+
 # FIXME: The user object could simply be a str (the username)
 # We need this because HomeHandler._start_container takes an object with
 # the username in its `name` attribute
@@ -80,9 +84,9 @@ def __init__(self, id, name):
             'policy.volume_mode')
 
 
-@mergedocs(ABCAccounting)
-class CSVAccounting(ABCAccounting):
-    """ Accounting class that reads a CSV file and is used by the
+@mergedocs(ABCDatabase)
+class CSVDatabase(ABCDatabase):
+    """ Database class that reads a CSV file and is used by the
     remoteappmanager.  Currently only accepts one csv file.
     """
 
@@ -167,10 +171,13 @@ def __init__(self, csv_file_path, **kwargs):
 
                 # Save the configuration
                 # Note that we don't filter existing duplicate entry
-                self.all_records.setdefault(user.name, []).append((
-                     uuid.uuid4().hex,
-                     application,
-                     application_policy))
+                self.all_records.setdefault(user.name, []).append(
+                    CSVAccounting(
+                        id=uuid.uuid4().hex,
+                        user=user,
+                        application=application,
+                        application_policy=application_policy)
+                )
 
     def get_user(self, *, user_name=None, id=None):
         if not one([user_name, id]):
@@ -183,11 +190,11 @@ def get_user(self, *, user_name=None, id=None):
         else:
             raise RuntimeError("Impossible condition")  # pragma: no cover
 
-    def get_apps_for_user(self, user):
+    def get_accounting_for_user(self, user):
         if user is not None:
-            return tuple(self.all_records.get(user.name, ()))
+            return self.all_records.get(user.name, [])
         else:
-            return ()
+            return []
 
     def create_user(self, user_name):
         raise UnsupportedOperation()

remoteappmanager/db/interfaces.py

@@ -57,6 +57,14 @@ def __repr__(self):
 
 
 class ABCAccounting(metaclass=ABCMeta):
+    def __init__(self, id, user, application, application_policy):
+        self.id = id
+        self.user = user
+        self.application = application
+        self.application_policy = application_policy
+
+
+class ABCDatabase(metaclass=ABCMeta):
     """ Main accounting interface required by the single user application.
     """
 
@@ -80,8 +88,8 @@ def get_user(self, *, user_name=None, id=None):
         """
 
     @abstractmethod
-    def get_apps_for_user(self, user):
-        """ Return an iterable of ApplicationConfig for a given user
+    def get_accounting_for_user(self, user):
+        """ Returns the accounting information for a given user
 
         Parameters
         ----------
@@ -90,10 +98,9 @@ def get_apps_for_user(self, user):
 
         Returns
         -------
-        tuple
-           each item of the tuple should be a tuple of
-           (id, ABCApplication, ABCApplicationPolicy) where id is a string
-           used for identifying (ABCApplication, ABCApplicationPolicy)
+        list
+           each item of the list should be an instance satisfying the
+           ABCAccounting format (duck typing)
         """
 
     @abstractmethod

remoteappmanager/db/orm.py

@@ -8,11 +8,11 @@
 from sqlalchemy.engine import Engine
 from sqlalchemy.exc import OperationalError, IntegrityError
 from sqlalchemy.ext.declarative import declarative_base
-from sqlalchemy.orm import sessionmaker, relationship
+from sqlalchemy.orm import sessionmaker, relationship, joinedload
 from sqlalchemy.orm.exc import DetachedInstanceError, NoResultFound
 
 from remoteappmanager.logging.logging_mixin import LoggingMixin
-from remoteappmanager.db.interfaces import ABCAccounting
+from remoteappmanager.db.interfaces import ABCDatabase
 from remoteappmanager.db import exceptions
 from remoteappmanager.utils import parse_volume_string, mergedocs, one
 
@@ -163,8 +163,8 @@ def reset(self):
         Base.metadata.create_all(self.engine)
 
 
-@mergedocs(ABCAccounting)
-class AppAccounting(ABCAccounting):
+@mergedocs(ABCDatabase)
+class ORMDatabase(ABCDatabase):
 
     def __init__(self, url, **kwargs):
         ''' Initialiser
@@ -226,11 +226,11 @@ def get_user(self, *, user_name=None, id=None):
 
         return user
 
-    def get_apps_for_user(self, user):
+    def get_accounting_for_user(self, user):
         # We create a session here to make sure it is only
         # used in one thread
         with contextlib.closing(self.db.create_session()) as session:
-            result = apps_for_user(session, user)
+            result = accounting_for_user(session, user)
 
             # Removing internal references to the session is
             # required such that the objects can be reused
@@ -436,25 +436,28 @@ def transaction(session):
         session.commit()
 
 
-def apps_for_user(session, user):
-    """Returns a tuple of tuples, each containing an application and the
-    associated policy that the specified orm user is allowed to run.
+def accounting_for_user(session, user):
+    """Returns a list of Accounting objects, each containing
+    an application and the associated policy that the specified orm user is
+    allowed to run.
     If the user is None, the default is to return an empty list.
-    The mapping_id is a unique string identifying the combination of
+    The id is a unique string identifying the combination of
     application and policy. It is not unique per user.
+
     Parameters
     ----------
     session : Session
         The current session
     user : User or None
         the orm User, or None.
+
     Returns
     -------
-    A tuple of tuples (mapping_id, orm.Application, orm.ApplicationPolicy)
+    A list of Accounting objects
     """
 
     if user is None:
-        return ()
+        return []
 
     try:
         user_name = user.name
@@ -466,10 +469,10 @@ def apps_for_user(session, user):
         session.add(user)
         user_name = user.name
 
-    res = session.query(Accounting).join(
-        Accounting.user, aliased=True).filter_by(
-            name=user_name).all()
+    res = session.query(Accounting) \
+        .join(Accounting.user, aliased=True) \
+        .options(joinedload(Accounting.application)) \
+        .options(joinedload(Accounting.application_policy)) \
+        .filter_by(name=user_name).all()
 
-    return tuple(((acc.id,
-                   acc.application,
-                   acc.application_policy) for acc in res))
+    return res