Repository: simphony/simphony-remote

Issue:
User.orm_user should be renamed as User.account with an opaque type
The `orm_user` attribute of the singleuser app is an opaque object handled by the database's accounting.
It should be renamed as `account` and be given an opaque type.


Discussion:

Patch:
diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py
index 89ba765..b5131db 100644
--- a/remoteappmanager/application.py
+++ b/remoteappmanager/application.py
@@ -125,7 +125,7 @@ class Application(web.Application, LoggingMixin):
         """Initializes the user at the database level."""
         user_name = self.command_line_config.user
         user = User(name=user_name)
-        user.orm_user = self.db.get_user_by_name(user_name)
+        user.account = self.db.get_user_by_name(user_name)
         return user
 
     # Public
diff --git a/remoteappmanager/db/csv_db.py b/remoteappmanager/db/csv_db.py
index 3c00a2e..99227dc 100644
--- a/remoteappmanager/db/csv_db.py
+++ b/remoteappmanager/db/csv_db.py
@@ -158,7 +158,7 @@ class CSVAccounting(ABCAccounting):
 
         Parameters
         ----------
-        user : remoteappmanager.db.csv_db.CSVUser
+        user : CSVUser
            Same type as the result of `get_user_by_name`
 
         Returns
diff --git a/remoteappmanager/db/interfaces.py b/remoteappmanager/db/interfaces.py
index e3c7485..4f09018 100644
--- a/remoteappmanager/db/interfaces.py
+++ b/remoteappmanager/db/interfaces.py
@@ -55,7 +55,7 @@ class ABCApplicationPolicy(metaclass=ABCMeta):
 
 
 class ABCAccounting(metaclass=ABCMeta):
-    """ Main accounting interface required by the single User application.
+    """ Main accounting interface required by the single user application.
     """
 
     @abstractmethod
@@ -69,16 +69,17 @@ class ABCAccounting(metaclass=ABCMeta):
 
         Returns
         -------
-        a User-like object that the Database understands
+        account : opaque-type
+           an object that the database understands
         """
 
     @abstractmethod
-    def get_apps_for_user(self, user):
-        """ Return an iterable of ApplicationConfig for a given User
+    def get_apps_for_user(self, account):
+        """ Return an iterable of ApplicationConfig for a given account
 
         Parameters
         ----------
-        user : User-like
+        account : opaque-type
            Same type as the result of `get_user_by_name`
 
         Returns
diff --git a/remoteappmanager/handlers/home_handler.py b/remoteappmanager/handlers/home_handler.py
index 0ec4c4e..9ba81f6 100644
--- a/remoteappmanager/handlers/home_handler.py
+++ b/remoteappmanager/handlers/home_handler.py
@@ -78,7 +78,7 @@ class HomeHandler(BaseHandler):
         mapping_id = options["mapping_id"][0]
 
         all_apps = self.application.db.get_apps_for_user(
-            self.current_user.orm_user)
+            self.current_user.account)
 
         choice = [(m_id, app, policy)
                   for m_id, app, policy in all_apps
@@ -90,9 +90,9 @@ class HomeHandler(BaseHandler):
         _, app, policy = choice[0]
 
         container = None
-        orm_user = self.current_user.orm_user
+        user_name = self.current_user.name
         try:
-            container = yield self._start_container(orm_user,
+            container = yield self._start_container(user_name,
                                                     app,
                                                     policy,
                                                     mapping_id)
@@ -172,7 +172,7 @@ class HomeHandler(BaseHandler):
         container_manager = self.application.container_manager
 
         apps = self.application.db.get_apps_for_user(
-            self.current_user.orm_user)
+            self.current_user.account)
 
         images_info = []
 
@@ -230,18 +230,16 @@ class HomeHandler(BaseHandler):
 
         return Container.from_docker_containers_dict(container_dict[0])
 
-    # FIXME: The orm_user here requires any database implementation
-    # to provide a user object with a name attribute
     @gen.coroutine
-    def _start_container(self, orm_user, app, policy, mapping_id):
+    def _start_container(self, user_name, app, policy, mapping_id):
         """Start the container. This method is a helper method that
         works with low level data and helps in issuing the request to the
         data container.
 
         Parameters
         ----------
-        orm_user : User
-            database's user object (e.g. current_user.orm_user)
+        user_name : str
+            the user name to be associated with the container
 
         app : ABCApplication
             the application to start
@@ -254,7 +252,6 @@ class HomeHandler(BaseHandler):
         Container
         """
 
-        user_name = orm_user.name
         image_name = app.image
         mount_home = policy.allow_home
         volume_spec = (policy.volume_source,
diff --git a/remoteappmanager/user.py b/remoteappmanager/user.py
index 50e3428..4820d5b 100644
--- a/remoteappmanager/user.py
+++ b/remoteappmanager/user.py
@@ -8,7 +8,5 @@ class User(HasTraits):
     # The username as passed at the config line
     name = Unicode()
 
-    # FIXME: orm_user is Any to support other database implementation
-
     #: Can be none if the username cannot be found in the database.
-    orm_user = Any()
+    account = Any()


Test patch:
diff --git a/tests/test_application.py b/tests/test_application.py
index eee2f8b..c51080d 100644
--- a/tests/test_application.py
+++ b/tests/test_application.py
@@ -44,7 +44,7 @@ class TestApplication(TempMixin, testing.AsyncTestCase):
         self.assertIsNotNone(app.container_manager)
         self.assertIsNotNone(app.hub)
         self.assertEqual(app.user.name, "username")
-        self.assertEqual(app.user.orm_user, None)
+        self.assertEqual(app.user.account, None)
 
 
 # FIXME: Some of these tests are the same and should be refactored
@@ -89,4 +89,4 @@ class TestApplicationWithCSV(TempMixin, testing.AsyncTestCase):
         self.assertIsNotNone(app.user)
 
         self.assertEqual(app.user.name, "username")
-        self.assertIsInstance(app.user.orm_user, test_csv_db.CSVUser)
+        self.assertIsInstance(app.user.account, test_csv_db.CSVUser)