From ce30bd70587884efd570d17c853bd9ac8e881859 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 20 Apr 2017 13:52:47 +0100
Subject: [PATCH 01/12] Updating to new version

---
 remoteappmanager/admin_application.py         |  8 +--
 remoteappmanager/application.py               |  4 +-
 remoteappmanager/base_application.py          |  4 +-
 remoteappmanager/webapi/__init__.py           |  4 +-
 remoteappmanager/webapi/admin/__init__.py     |  8 +--
 remoteappmanager/webapi/admin/accounting.py   | 47 +++++++------
 remoteappmanager/webapi/admin/application.py  | 31 +++++----
 remoteappmanager/webapi/admin/container.py    | 24 +++++--
 remoteappmanager/webapi/admin/user.py         | 28 ++++----
 remoteappmanager/webapi/application.py        | 58 ++++++++++++----
 remoteappmanager/webapi/container.py          | 69 ++++++++++---------
 .../webapi/tests/test_application.py          |  4 +-
 .../webapi/tests/test_container.py            | 19 +++--
 requirements.txt                              |  2 +-
 14 files changed, 183 insertions(+), 127 deletions(-)

diff --git a/remoteappmanager/admin_application.py b/remoteappmanager/admin_application.py
index c83a9b68c..2753dd502 100644
--- a/remoteappmanager/admin_application.py
+++ b/remoteappmanager/admin_application.py
@@ -15,10 +15,10 @@ class AdminApplication(BaseApplication):
     """Tornado main application"""
 
     def _webapi_resources(self):
-        return [admin.Container,
-                admin.Application,
-                admin.User,
-                admin.Accounting]
+        return [admin.ContainerHandler,
+                admin.ApplicationHandler,
+                admin.UserHandler,
+                admin.AccountingHandler]
 
     def _web_handlers(self):
         base_urlpath = self.command_line_config.base_urlpath
diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py
index 50bc36ab6..4577c5e1c 100644
--- a/remoteappmanager/application.py
+++ b/remoteappmanager/application.py
@@ -11,8 +11,8 @@ class Application(BaseApplication):
     """Tornado main application"""
 
     def _webapi_resources(self):
-        return [webapi.Application,
-                webapi.Container]
+        return [webapi.ApplicationHandler,
+                webapi.ContainerHandler]
 
     def _web_handlers(self):
         base_urlpath = self.command_line_config.base_urlpath
diff --git a/remoteappmanager/base_application.py b/remoteappmanager/base_application.py
index fd9e25bf5..11496e1a1 100644
--- a/remoteappmanager/base_application.py
+++ b/remoteappmanager/base_application.py
@@ -141,8 +141,8 @@ def _user_default(self):
     def _registry_default(self):
         reg = Registry()
         reg.authenticator = HubAuthenticator
-        for resource_class in self._webapi_resources():
-            reg.register(resource_class)
+        for resource_handler in self._webapi_resources():
+            reg.register(resource_handler)
         return reg
 
     # Public
diff --git a/remoteappmanager/webapi/__init__.py b/remoteappmanager/webapi/__init__.py
index 207e531ed..5ccd6ed30 100644
--- a/remoteappmanager/webapi/__init__.py
+++ b/remoteappmanager/webapi/__init__.py
@@ -1,2 +1,2 @@
-from .container import Container  # noqa
-from .application import Application  # noqa
+from .container import ContainerHandler  # noqa
+from .application import ApplicationHandler  # noqa
diff --git a/remoteappmanager/webapi/admin/__init__.py b/remoteappmanager/webapi/admin/__init__.py
index 62b9771c5..fa0d7934e 100644
--- a/remoteappmanager/webapi/admin/__init__.py
+++ b/remoteappmanager/webapi/admin/__init__.py
@@ -1,4 +1,4 @@
-from .container import Container  # noqa
-from .application import Application  # noqa
-from .user import User  # noqa
-from .accounting import Accounting  # noqa
+from .container import ContainerHandler  # noqa
+from .application import ApplicationHandler  # noqa
+from .user import UserHandler  # noqa
+from .accounting import AccountingHandler  # noqa
diff --git a/remoteappmanager/webapi/admin/accounting.py b/remoteappmanager/webapi/admin/accounting.py
index f39a858f0..b16e2426e 100644
--- a/remoteappmanager/webapi/admin/accounting.py
+++ b/remoteappmanager/webapi/admin/accounting.py
@@ -1,4 +1,6 @@
 from tornado import gen
+from tornadowebapi.resource_handler import ResourceHandler
+from tornadowebapi.traitlets import Absent, Unicode, Bool
 
 from remoteappmanager.utils import parse_volume_string
 from tornadowebapi import exceptions
@@ -9,47 +11,48 @@
 
 
 class Accounting(Resource):
-    __collection_name__ = "accounting"
-
-    def validate_representation(self, representation):
-        representation["user_name"] = _not_empty_str(
-            representation["user_name"])
-        representation["image_name"] = _not_empty_str(
-            representation["image_name"])
-        representation["allow_home"] = bool(
-            representation["allow_home"])
-
-        if "volume" in representation:
-            representation["volume"] = _not_empty_str(
-                representation["volume"])
-            parse_volume_string(representation["volume"])
-        return representation
+    user_name = Unicode()
+    image_name = Unicode()
+    allow_home = Bool()
+    volume = Unicode(optional=True)
+
+    @classmethod
+    def collection_name(cls):
+        return "accounting"
+
+
+class AccountingHandler(ResourceHandler):
+    resource_class = Accounting
 
     @gen.coroutine
     @authenticated
-    def create(self, representation):
+    def create(self, resource, **kwargs):
         db = self.application.db
 
+        volume = {} if resource.volume == Absent else resource.volume
+        parse_volume_string(volume)
+
         try:
             id = db.grant_access(
-                representation["image_name"],
-                representation["user_name"],
-                representation["allow_home"],
+                resource.image_name,
+                resource.user_name,
+                resource.allow_home,
                 True,
-                representation.get("volume")
+                volume,
                 )
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
 
+        resource.identifier = id
         return id
 
     @gen.coroutine
     @authenticated
-    def delete(self, identifier):
+    def delete(self, resource, **kwargs):
         db = self.application.db
 
         try:
-            db.revoke_access_by_id(identifier)
+            db.revoke_access_by_id(resource.identifier)
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
 
diff --git a/remoteappmanager/webapi/admin/application.py b/remoteappmanager/webapi/admin/application.py
index 6e794774f..a4a0a80b1 100644
--- a/remoteappmanager/webapi/admin/application.py
+++ b/remoteappmanager/webapi/admin/application.py
@@ -2,31 +2,34 @@
 
 from tornadowebapi import exceptions
 from tornadowebapi.resource import Resource
+from tornadowebapi.resource_handler import ResourceHandler
+from tornadowebapi.traitlets import Unicode
 
 from remoteappmanager.webapi.decorators import authenticated
 from remoteappmanager.db import exceptions as db_exceptions
 
 
 class Application(Resource):
-    def validate_representation(self, representation):
-        representation["image_name"] = str(representation["image_name"])
-        if len(representation["image_name"]) == 0:
-            raise ValueError("image_name cannot be empty")
+    image_name = Unicode(allow_empty=False, strip=True)
 
-        return representation
 
-    def validate_identifier(self, identifier):
-        return int(identifier)
+class ApplicationHandler(ResourceHandler):
+    resource_class = Application
 
     @gen.coroutine
     @authenticated
-    def delete(self, identifier):
+    def delete(self, resource, **kwargs):
         """Removes the application."""
         db = self.application.db
+        try:
+            id = int(resource.identifier)
+        except ValueError:
+            raise exceptions.NotFound()
 
         try:
-            db.remove_application(id=identifier)
-            self.log.info("Removed application with id {}".format(identifier))
+            db.remove_application(id=id)
+            self.log.info("Removed application with id {}".format(
+                resource.identifier))
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
         except db_exceptions.UnsupportedOperation:
@@ -34,15 +37,13 @@ def delete(self, identifier):
 
     @gen.coroutine
     @authenticated
-    def create(self, representation):
-        image_name = representation["image_name"]
-
+    def create(self, resource, **kwargs):
         db = self.application.db
         try:
-            id = db.create_application(image_name)
+            id = db.create_application(resource.image_name)
         except db_exceptions.Exists:
             raise exceptions.Exists()
         except db_exceptions.UnsupportedOperation:
             raise exceptions.Unable()
 
-        return id
+        resource.identifier = str(id)
diff --git a/remoteappmanager/webapi/admin/container.py b/remoteappmanager/webapi/admin/container.py
index 32d9bf3f4..94de0bef5 100644
--- a/remoteappmanager/webapi/admin/container.py
+++ b/remoteappmanager/webapi/admin/container.py
@@ -2,21 +2,29 @@
 
 from tornadowebapi import exceptions
 from tornadowebapi.resource import Resource
+from tornadowebapi.resource_handler import ResourceHandler
 
 from remoteappmanager.webapi.decorators import authenticated
 
 
 class Container(Resource):
+    pass
+
+
+class ContainerHandler(ResourceHandler):
+    resource_class = Container
+
     @gen.coroutine
     @authenticated
-    def delete(self, identifier):
+    def delete(self, resource, **kwargs):
         """Stop the container."""
         container_manager = self.application.container_manager
-        container = yield container_manager.find_container(url_id=identifier)
+        container = yield container_manager.find_container(
+            url_id=resource.identifier)
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
-                identifier))
+                resource.identifier))
             raise exceptions.NotFound()
 
         try:
@@ -25,12 +33,14 @@ def delete(self, identifier):
             # If we can't remove the reverse proxy, we cannot do much more
             # than log the problem and keep going, because we want to stop
             # the container regardless.
-            self.log.exception("Could not remove reverse "
-                               "proxy for id {}".format(identifier))
+            self.log.exception(
+                "Could not remove reverse proxy for id {}".format(
+                    resource.identifier))
 
         try:
             yield container_manager.stop_and_remove_container(
                 container.docker_id)
         except Exception:
-            self.log.exception("Could not stop and remove container "
-                               "for id {}".format(identifier))
+            self.log.exception(
+                "Could not stop and remove container for id {}".format(
+                    resource.identifier))
diff --git a/remoteappmanager/webapi/admin/user.py b/remoteappmanager/webapi/admin/user.py
index 3eb7cdcbc..dc9222244 100644
--- a/remoteappmanager/webapi/admin/user.py
+++ b/remoteappmanager/webapi/admin/user.py
@@ -2,29 +2,29 @@
 
 from tornadowebapi import exceptions
 from tornadowebapi.resource import Resource
+from tornadowebapi.resource_handler import ResourceHandler
+from tornadowebapi.traitlets import Unicode
 
 from remoteappmanager.webapi.decorators import authenticated
 from remoteappmanager.db import exceptions as db_exceptions
 
 
 class User(Resource):
-    def validate_representation(self, representation):
-        representation["name"] = str(representation["name"]).strip()
-        if len(representation["name"]) == 0:
-            raise ValueError("name cannot be empty")
-        return representation
+    name = Unicode(allow_empty=False, strip=True)
 
-    def validate_identifier(self, identifier):
-        return int(identifier)
+
+class UserHandler(ResourceHandler):
+    resource_class = User
 
     @gen.coroutine
     @authenticated
-    def delete(self, identifier):
+    def delete(self, resource, **kwargs):
         db = self.application.db
 
         try:
-            db.remove_user(id=identifier)
-            self.log.info("Removed user with id {}".format(identifier))
+            db.remove_user(id=int(resource.identifier))
+            self.log.info("Removed user with id {}".format(
+                resource.identifier))
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
         except db_exceptions.UnsupportedOperation:
@@ -32,15 +32,13 @@ def delete(self, identifier):
 
     @gen.coroutine
     @authenticated
-    def create(self, representation):
-        name = representation["name"]
+    def create(self, resource, **kwargs):
+        name = resource.name
 
         db = self.application.db
         try:
-            id = db.create_user(name)
+            resource.identifier = str(db.create_user(name))
         except db_exceptions.Exists:
             raise exceptions.Exists()
         except db_exceptions.UnsupportedOperation:
             raise exceptions.Unable()
-
-        return id
diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index 6ddaf00df..4da30244e 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -1,17 +1,51 @@
 from tornado import gen
 from tornadowebapi.exceptions import NotFound
 from tornadowebapi.resource import Resource
+from tornadowebapi.resource_fragment import ResourceFragment
+from tornadowebapi.traitlets import Unicode, OneOf, Bool, List
+from tornadowebapi.resource_handler import ResourceHandler
 
 from remoteappmanager.webapi.decorators import authenticated
 
 
+class Container(ResourceFragment):
+    name = Unicode()
+    image_name = Unicode()
+    url_id = Unicode()
+
+
+class Policy(ResourceFragment):
+    allow_home = Bool()
+    volume_source = Unicode()
+    volume_target = Unicode()
+    volume_mode = Unicode()
+
+
+class Image(ResourceFragment):
+    name = Unicode()
+    ui_name = Unicode()
+    icon_128 = Unicode()
+    description = Unicode()
+    policy = OneOf(Policy)
+    configurables = List(Unicode)
+
+
 class Application(Resource):
+    image = OneOf(Image)
+    mapping_id = Unicode()
+    container = OneOf(Container)
+
+
+class ApplicationHandler(ResourceHandler):
+    resource_class = Application
+
     @gen.coroutine
     @authenticated
-    def retrieve(self, identifier):
+    def retrieve(self, instance):
         apps = self.application.db.get_apps_for_user(
             self.current_user.account
         )
+        identifier = instance.identifier
 
         # Convert the list of tuples in a dict
         apps_dict = {mapping_id: (app, policy)
@@ -33,7 +67,7 @@ def retrieve(self, identifier):
             user_name=self.current_user.name,
             mapping_id=identifier)
 
-        representation = {
+        instance.fill({
             "image": {
                 "name": image.name,
                 "ui_name": image.ui_name,
@@ -48,25 +82,21 @@ def retrieve(self, identifier):
                 "configurables": [conf.tag for conf in image.configurables]
             },
             "mapping_id": identifier,
-        }
+        })
 
         if len(containers):
             # We assume that we can only run one container only (although the
             # API considers a broader possibility for future extension.
             container = containers[0]
-            representation["container"] = dict(
-                name=container.name,
-                image_name=container.image_name,
-                url_id=container.url_id,
-            )
-        else:
-            representation["container"] = None
-
-        return representation
+            instance.container = Container().fill({
+                "name": container.name,
+                "image_name": container.image_name,
+                "url_id": container.url_id,
+            })
 
     @gen.coroutine
     @authenticated
-    def items(self):
+    def items(self, items_response):
         """Retrieves a dictionary containing the image and the associated
         container, if active, as values."""
         apps = self.application.db.get_apps_for_user(self.current_user.account)
@@ -78,4 +108,4 @@ def items(self):
             if (yield container_manager.image(app.image)) is not None:
                 result.append(mapping_id)
 
-        return result
+        items_response.set(result)
diff --git a/remoteappmanager/webapi/container.py b/remoteappmanager/webapi/container.py
index 85475c6c8..1f6e39ddf 100644
--- a/remoteappmanager/webapi/container.py
+++ b/remoteappmanager/webapi/container.py
@@ -5,28 +5,29 @@
 
 from tornadowebapi import exceptions
 from tornadowebapi.resource import Resource
+from tornadowebapi.resource_handler import ResourceHandler
+from tornadowebapi.traitlets import Unicode, Dict, Absent
 
 from remoteappmanager.netutils import wait_for_http_server_2xx
 from remoteappmanager.webapi.decorators import authenticated
 
 
 class Container(Resource):
-    def validate_representation(self, representation):
-        try:
-            representation["mapping_id"]
-        except KeyError:
-            raise exceptions.BadRepresentation(message="missing mapping_id")
+    mapping_id = Unicode()
+    name = Unicode()
+    image_name = Unicode()
+    configurables = Dict(optional=True)
+
 
-        return representation
+class ContainerHandler(ResourceHandler):
+    resource_class = Container
 
     @gen.coroutine
     @authenticated
-    def create(self, representation):
-        """Create the container.
-        The representation should accept the application mapping id we
-        want to start"""
+    def create(self, resource, **kwargs):
+        """Create the container."""
 
-        mapping_id = representation["mapping_id"]
+        mapping_id = resource.mapping_id
 
         webapp = self.application
         account = self.current_user.account
@@ -50,8 +51,7 @@ def create(self, representation):
             raise exceptions.BadRepresentation(message="unrecognized image")
 
         try:
-            environment = self._environment_from_configurables(image,
-                                                               representation)
+            environment = self._environment_from_configurables(image, resource)
         except Exception:
             raise exceptions.BadRepresentation(message="invalid configurables")
 
@@ -69,7 +69,7 @@ def create(self, representation):
             raise exceptions.Unable(message=str(e))
 
         try:
-                yield self._wait_for_container_ready(container)
+            yield self._wait_for_container_ready(container)
         except Exception as e:
             self._remove_container_noexcept(container)
             raise exceptions.Unable(message=str(e))
@@ -82,40 +82,41 @@ def create(self, representation):
             self._remove_container_noexcept(container)
             raise exceptions.Unable(message=str(e))
 
-        return container.url_id
+        resource.identifier = container.url_id
 
     @gen.coroutine
     @authenticated
-    def retrieve(self, identifier):
+    def retrieve(self, resource, **kwargs):
         """Return the representation of the running container."""
         container_manager = self.application.container_manager
         container = yield container_manager.find_container(
-            url_id=identifier,
+            url_id=resource.identifier,
             user_name=self.current_user.name)
 
         if container is None:
             self.log.warning("Could not find container for id {}".format(
-                identifier))
+                resource.identifier))
             raise exceptions.NotFound()
 
-        return dict(
+        return resource.fill(dict(
             name=container.name,
-            image_name=container.image_name
-        )
+            image_name=container.image_name,
+            mapping_id=container.mapping_id,
+        ))
 
     @gen.coroutine
     @authenticated
-    def delete(self, identifier):
+    def delete(self, resource, **kwargs):
         """Stop the container."""
         container_manager = self.application.container_manager
         container = yield container_manager.find_container(
-            url_id=identifier,
+            url_id=resource.identifier,
             user_name=self.current_user.name
         )
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
-                             identifier))
+                             resource.identifier))
             raise exceptions.NotFound()
 
         try:
@@ -127,18 +128,18 @@ def delete(self, identifier):
             # than log the problem and keep going, because we want to stop
             # the container regardless.
             self.log.exception("Could not remove reverse "
-                               "proxy for id {}".format(identifier))
+                               "proxy for id {}".format(resource.identifier))
 
         try:
             yield container_manager.stop_and_remove_container(
                 container.docker_id)
         except Exception:
             self.log.exception("Could not stop and remove container "
-                               "for id {}".format(identifier))
+                               "for id {}".format(resource.identifier))
 
     @gen.coroutine
     @authenticated
-    def items(self):
+    def items(self, items_response, **kwargs):
         """"Return the list of containers we are currently running."""
         container_manager = self.application.container_manager
 
@@ -159,9 +160,11 @@ def items(self):
                 user_name=self.current_user.name,
                 mapping_id=mapping_id)
 
-            running_containers.append(container.url_id)
+            rest_container = Container(identifier=container.url_id)
+            rest_container.fill(container)
+            running_containers.append(rest_container)
 
-        return running_containers
+        items_response.set(running_containers)
 
     ##################
     # Private
@@ -269,16 +272,18 @@ def _start_container(self,
 
         return container
 
-    def _environment_from_configurables(self, image, representation):
+    def _environment_from_configurables(self, image, resource):
         """Helper routine: extracts the configurables from the
         image, matches them to the appropriate configurables
         data in the representation, and returns the resulting environment
         """
         env = {}
 
+        if image.configurables == Absent:
+            return env
+
         for img_conf in image.configurables:
-            config_dict = representation.get(
-                "configurables", {}).get(img_conf.tag, {})
+            config_dict = resource.configurables.get(img_conf.tag)
             env.update(img_conf.config_dict_to_env(config_dict))
 
         return env
diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 0163bca62..b1093233e 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -9,7 +9,7 @@
 
 from remoteappmanager.docker.container import Container
 from remoteappmanager.docker.image import Image
-from remoteappmanager.webapi import Application
+from remoteappmanager.webapi import ApplicationHandler
 from remoteappmanager.tests.webapi_test_case import WebAPITestCase
 from remoteappmanager.tests.mocking.dummy import create_hub
 
@@ -29,7 +29,7 @@ def setUp(self):
 
     def get_app(self):
         self.reg = registry.Registry()
-        self.reg.register(Application)
+        self.reg.register(ApplicationHandler)
         self.reg.authenticator = DummyAuthenticator
         handlers = self.reg.api_handlers('/')
         app = web.Application(handlers=handlers)
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index 75934b148..d750659df 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -200,19 +200,28 @@ def test_create_fails_for_missing_mapping_id(self):
         _, data = self.post(
             "/user/johndoe/api/v1/containers/",
             dict(
-                whatever="123"
+                name="123",
+                configurables={},
+                image_name="456",
             ),
             httpstatus.BAD_REQUEST
         )
 
-        self.assertEqual(data,
-                         {"type": "BadRepresentation",
-                          "message": "missing mapping_id"})
+        self.assertEqual(
+            data,
+            {"type": "BadRepresentation",
+             "message": "missing mandatory elements: {'mapping_id'}"
+             })
 
     def test_create_fails_for_invalid_mapping_id(self):
         _, data = self.post(
             "/user/johndoe/api/v1/containers/",
-            dict(mapping_id="whatever"),
+            dict(
+                mapping_id="whatever",
+                name="123",
+                configurables={},
+                image_name="456",
+            ),
             httpstatus.BAD_REQUEST
         )
 
diff --git a/requirements.txt b/requirements.txt
index 087d417cd..98e630513 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -11,5 +11,5 @@ git+git://github.com/jupyterhub/jupyterhub.git@2d1a45f0190059ef436c2f97dc8d6e391
 jupyter_client==4.3.0
 click==6.6
 tabulate==0.7.5
-git+git://github.com/simphony/tornado-webapi.git@v0.5.0#egg=tornadowebapi
+git+git://github.com/simphony/tornado-webapi.git@master#egg=tornadowebapi
 oauthenticator==0.5.1

From 88f112993b972442292e90926173e9636ef3ac9d Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 21 Apr 2017 10:44:38 +0100
Subject: [PATCH 02/12] Other changes to adapt to new tornado-webapi

---
 remoteappmanager/tests/webapi_test_case.py    |  4 ++-
 remoteappmanager/webapi/admin/user.py         |  6 +++-
 remoteappmanager/webapi/application.py        |  7 +++--
 remoteappmanager/webapi/container.py          | 10 +++----
 .../webapi/tests/test_application.py          | 10 +++----
 .../webapi/tests/test_container.py            | 30 +++++++++++++++----
 6 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/remoteappmanager/tests/webapi_test_case.py b/remoteappmanager/tests/webapi_test_case.py
index 0d1ded7f2..fefdd3426 100644
--- a/remoteappmanager/tests/webapi_test_case.py
+++ b/remoteappmanager/tests/webapi_test_case.py
@@ -1,8 +1,10 @@
 from tornado import escape
+from tornado.testing import LogTrapTestCase
+
 from .utils import AsyncHTTPTestCase
 
 
-class WebAPITestCase(AsyncHTTPTestCase):
+class WebAPITestCase(AsyncHTTPTestCase, LogTrapTestCase):
     """Base class for tests accessing the Web API,
     providing basic methods to issue web requests, get results, and compare
     against the expected HTTP code.
diff --git a/remoteappmanager/webapi/admin/user.py b/remoteappmanager/webapi/admin/user.py
index dc9222244..0791d0f3f 100644
--- a/remoteappmanager/webapi/admin/user.py
+++ b/remoteappmanager/webapi/admin/user.py
@@ -20,9 +20,13 @@ class UserHandler(ResourceHandler):
     @authenticated
     def delete(self, resource, **kwargs):
         db = self.application.db
+        try:
+            identifier = int(resource.identifier)
+        except ValueError:
+            raise exceptions.NotFound()
 
         try:
-            db.remove_user(id=int(resource.identifier))
+            db.remove_user(id=identifier)
             self.log.info("Removed user with id {}".format(
                 resource.identifier))
         except db_exceptions.NotFound:
diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index 4da30244e..58d71bbff 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -32,8 +32,7 @@ class Image(ResourceFragment):
 
 class Application(Resource):
     image = OneOf(Image)
-    mapping_id = Unicode()
-    container = OneOf(Container)
+    container = OneOf(Container, optional=True)
 
 
 class ApplicationHandler(ResourceHandler):
@@ -106,6 +105,8 @@ def items(self, items_response):
         result = []
         for mapping_id, app, policy in apps:
             if (yield container_manager.image(app.image)) is not None:
-                result.append(mapping_id)
+                resource = self.resource_class(identifier=mapping_id)
+
+                result.append(resource)
 
         items_response.set(result)
diff --git a/remoteappmanager/webapi/container.py b/remoteappmanager/webapi/container.py
index 1f6e39ddf..cd8a60d8d 100644
--- a/remoteappmanager/webapi/container.py
+++ b/remoteappmanager/webapi/container.py
@@ -13,10 +13,10 @@
 
 
 class Container(Resource):
-    mapping_id = Unicode()
-    name = Unicode()
-    image_name = Unicode()
-    configurables = Dict(optional=True)
+    mapping_id = Unicode(allow_empty=False, strip=True)
+    configurables = Dict(optional=True, scope="input")
+    name = Unicode(scope="output", allow_empty=False, strip=True)
+    image_name = Unicode(scope="output", allow_empty=False, strip=True)
 
 
 class ContainerHandler(ResourceHandler):
@@ -279,7 +279,7 @@ def _environment_from_configurables(self, image, resource):
         """
         env = {}
 
-        if image.configurables == Absent:
+        if resource.configurables is Absent:
             return env
 
         for img_conf in image.configurables:
diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index b1093233e..49c678ee1 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -78,19 +78,18 @@ def test_retrieve(self):
         _, data = self.get("/api/v1/applications/one/", httpstatus.OK)
 
         self.assertEqual(data,
-                         {'container': None,
-                          'image': {
+                         {'image': {
+                              'configurables': [],
                               'description': '',
                               'icon_128': '',
                               'name': 'boo',
-                              'ui_name': 'foo_ui',
                               'policy': {
                                     "allow_home": True,
                                     "volume_mode": 'ro',
                                     "volume_source": "foo",
                                     "volume_target": "bar",
                               },
-                              'configurables': []
+                              'ui_name': 'foo_ui',
                           },
                           'mapping_id': 'one'})
 
@@ -106,7 +105,8 @@ def test_retrieve(self):
                          {'container':
                              {'image_name': 'xxx',
                               'name': 'container',
-                              'url_id': 'yyy'},
+                              'url_id': 'yyy'
+                              },
                           'image': {'description': '',
                                     'icon_128': '',
                                     'name': 'boo',
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index d750659df..429d5481c 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -26,13 +26,21 @@ def test_items(self):
         manager = self._app.container_manager
         manager.image = mock_coro_factory(Image())
         manager.find_containers = mock_coro_factory(
-            [DockerContainer()])
+            [
+                DockerContainer(user="johndoe",
+                                mapping_id="whatever",
+                                url_id="12345",
+                                name="container",
+                                image_name="image")
+                ])
 
         code, data = self.get(
             "/user/johndoe/api/v1/containers/",
             httpstatus.OK)
 
-        self.assertEqual(data, {"items": ["", ""]})
+        # We get two because we have two mapping ids, hence the find_containers
+        # gets called once per each mapping id.
+        self.assertEqual(data, {"items": ["12345", "12345"]})
 
     def test_create(self):
         with patch("remoteappmanager"
@@ -170,6 +178,8 @@ def test_create_succeeds_for_empty_configurable(self):
                 "/user/johndoe/api/v1/containers/",
                 dict(
                     mapping_id="cbaee2e8ef414f9fb0f1c97416b8aa6c",
+                    image_name="image",
+                    name="container",
                     configurables={
                         "resolution": {
                         }
@@ -181,6 +191,8 @@ def test_create_succeeds_for_empty_configurable(self):
             self.post(
                 "/user/johndoe/api/v1/containers/",
                 dict(
+                    image_name="image",
+                    name="container",
                     mapping_id="cbaee2e8ef414f9fb0f1c97416b8aa6c",
                     configurables={
                     }
@@ -188,9 +200,12 @@ def test_create_succeeds_for_empty_configurable(self):
                 httpstatus.CREATED
             )
 
+            print("YYY")
             self.post(
                 "/user/johndoe/api/v1/containers/",
                 dict(
+                    image_name="image",
+                    name="container",
                     mapping_id="cbaee2e8ef414f9fb0f1c97416b8aa6c",
                 ),
                 httpstatus.CREATED
@@ -210,7 +225,7 @@ def test_create_fails_for_missing_mapping_id(self):
         self.assertEqual(
             data,
             {"type": "BadRepresentation",
-             "message": "missing mandatory elements: {'mapping_id'}"
+             "message": "Missing mandatory elements: {'mapping_id'}"
              })
 
     def test_create_fails_for_invalid_mapping_id(self):
@@ -231,13 +246,16 @@ def test_create_fails_for_invalid_mapping_id(self):
 
     def test_retrieve(self):
         self._app.container_manager.find_container = mock_coro_factory(
-            DockerContainer(user="johndoe")
+            DockerContainer(user="johndoe",
+                            mapping_id="whatever",
+                            name="container",
+                            image_name="image")
         )
         _, data = self.get("/user/johndoe/api/v1/containers/found/",
                            httpstatus.OK)
 
-        self.assertEqual(data["image_name"], "")
-        self.assertEqual(data["name"], "")
+        self.assertEqual(data["image_name"], "image")
+        self.assertEqual(data["name"], "container")
 
         self._app.container_manager.find_container = \
             mock_coro_factory(return_value=None)

From 0c93d281be39065de5b247febc291c212da24b0a Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 21 Apr 2017 11:31:47 +0100
Subject: [PATCH 03/12] Fixed last tests

---
 remoteappmanager/webapi/admin/accounting.py   |  7 ++---
 remoteappmanager/webapi/admin/application.py  |  3 +-
 remoteappmanager/webapi/admin/container.py    |  9 +++---
 remoteappmanager/webapi/admin/user.py         |  3 +-
 remoteappmanager/webapi/application.py        | 28 ++++++-------------
 .../webapi/tests/test_application.py          |  7 ++---
 6 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/remoteappmanager/webapi/admin/accounting.py b/remoteappmanager/webapi/admin/accounting.py
index b16e2426e..1fc81bf44 100644
--- a/remoteappmanager/webapi/admin/accounting.py
+++ b/remoteappmanager/webapi/admin/accounting.py
@@ -11,10 +11,10 @@
 
 
 class Accounting(Resource):
-    user_name = Unicode()
-    image_name = Unicode()
+    user_name = Unicode(allow_empty=False, strip=True)
+    image_name = Unicode(allow_empty=False, strip=True)
     allow_home = Bool()
-    volume = Unicode(optional=True)
+    volume = Unicode(optional=True, allow_empty=False, strip=True)
 
     @classmethod
     def collection_name(cls):
@@ -44,7 +44,6 @@ def create(self, resource, **kwargs):
             raise exceptions.NotFound()
 
         resource.identifier = id
-        return id
 
     @gen.coroutine
     @authenticated
diff --git a/remoteappmanager/webapi/admin/application.py b/remoteappmanager/webapi/admin/application.py
index a4a0a80b1..024bdfcc9 100644
--- a/remoteappmanager/webapi/admin/application.py
+++ b/remoteappmanager/webapi/admin/application.py
@@ -28,8 +28,7 @@ def delete(self, resource, **kwargs):
 
         try:
             db.remove_application(id=id)
-            self.log.info("Removed application with id {}".format(
-                resource.identifier))
+            self.log.info("Removed application with id {}".format(id))
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
         except db_exceptions.UnsupportedOperation:
diff --git a/remoteappmanager/webapi/admin/container.py b/remoteappmanager/webapi/admin/container.py
index 94de0bef5..d1045dcf3 100644
--- a/remoteappmanager/webapi/admin/container.py
+++ b/remoteappmanager/webapi/admin/container.py
@@ -18,13 +18,12 @@ class ContainerHandler(ResourceHandler):
     @authenticated
     def delete(self, resource, **kwargs):
         """Stop the container."""
+        identifier = resource.identifier
         container_manager = self.application.container_manager
         container = yield container_manager.find_container(
-            url_id=resource.identifier)
+            url_id=identifier)
 
         if not container:
-            self.log.warning("Could not find container for id {}".format(
-                resource.identifier))
             raise exceptions.NotFound()
 
         try:
@@ -35,7 +34,7 @@ def delete(self, resource, **kwargs):
             # the container regardless.
             self.log.exception(
                 "Could not remove reverse proxy for id {}".format(
-                    resource.identifier))
+                    identifier))
 
         try:
             yield container_manager.stop_and_remove_container(
@@ -43,4 +42,4 @@ def delete(self, resource, **kwargs):
         except Exception:
             self.log.exception(
                 "Could not stop and remove container for id {}".format(
-                    resource.identifier))
+                    identifier))
diff --git a/remoteappmanager/webapi/admin/user.py b/remoteappmanager/webapi/admin/user.py
index 0791d0f3f..70b35f6e5 100644
--- a/remoteappmanager/webapi/admin/user.py
+++ b/remoteappmanager/webapi/admin/user.py
@@ -27,8 +27,7 @@ def delete(self, resource, **kwargs):
 
         try:
             db.remove_user(id=identifier)
-            self.log.info("Removed user with id {}".format(
-                resource.identifier))
+            self.log.info("Removed user with id {}".format(identifier))
         except db_exceptions.NotFound:
             raise exceptions.NotFound()
         except db_exceptions.UnsupportedOperation:
diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index 58d71bbff..71cd33757 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -40,11 +40,11 @@ class ApplicationHandler(ResourceHandler):
 
     @gen.coroutine
     @authenticated
-    def retrieve(self, instance):
+    def retrieve(self, resource, **kwargs):
         apps = self.application.db.get_apps_for_user(
             self.current_user.account
         )
-        identifier = instance.identifier
+        identifier = resource.identifier
 
         # Convert the list of tuples in a dict
         apps_dict = {mapping_id: (app, policy)
@@ -66,36 +66,26 @@ def retrieve(self, instance):
             user_name=self.current_user.name,
             mapping_id=identifier)
 
-        instance.fill({
-            "image": {
+        resource.image = Image()
+        resource.image.fill({
                 "name": image.name,
                 "ui_name": image.ui_name,
                 "icon_128": image.icon_128,
                 "description": image.description,
-                "policy": {
-                    "allow_home": policy.allow_home,
-                    "volume_source": policy.volume_source,
-                    "volume_target": policy.volume_target,
-                    "volume_mode": policy.volume_mode,
-                },
                 "configurables": [conf.tag for conf in image.configurables]
-            },
-            "mapping_id": identifier,
         })
+        resource.image.policy = Policy()
+        resource.image.policy.fill(policy)
 
         if len(containers):
             # We assume that we can only run one container only (although the
             # API considers a broader possibility for future extension.
-            container = containers[0]
-            instance.container = Container().fill({
-                "name": container.name,
-                "image_name": container.image_name,
-                "url_id": container.url_id,
-            })
+            resource.container = Container()
+            resource.container.fill(containers[0])
 
     @gen.coroutine
     @authenticated
-    def items(self, items_response):
+    def items(self, items_response, **kwargs):
         """Retrieves a dictionary containing the image and the associated
         container, if active, as values."""
         apps = self.application.db.get_apps_for_user(self.current_user.account)
diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 49c678ee1..9fc320469 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -90,8 +90,7 @@ def test_retrieve(self):
                                     "volume_target": "bar",
                               },
                               'ui_name': 'foo_ui',
-                          },
-                          'mapping_id': 'one'})
+                          }})
 
         self._app.container_manager.find_containers = \
             mock_coro_factory(return_value=[Container(
@@ -118,8 +117,8 @@ def test_retrieve(self):
                                         "volume_target": "bar",
                                     },
                                     'configurables': [],
-                                    },
-                          'mapping_id': 'one'})
+                                    }
+                          })
 
         self.get("/api/v1/applications/three/", httpstatus.NOT_FOUND)
 

From 36eb6a27fad70c30d0f0da120bce092a85b30f35 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 21 Apr 2017 12:45:54 +0100
Subject: [PATCH 04/12] Allowing None for volume info

---
 remoteappmanager/webapi/application.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index 71cd33757..a40b1eee4 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -16,9 +16,9 @@ class Container(ResourceFragment):
 
 class Policy(ResourceFragment):
     allow_home = Bool()
-    volume_source = Unicode()
-    volume_target = Unicode()
-    volume_mode = Unicode()
+    volume_source = Unicode(allow_none=True)
+    volume_target = Unicode(allow_none=True)
+    volume_mode = Unicode(allow_none=True)
 
 
 class Image(ResourceFragment):

From f56d235b9b8bd1a9c69e7e20f6333b61691ae34a Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 21 Apr 2017 16:44:59 +0100
Subject: [PATCH 05/12] changes to adapt to new item()

---
 remoteappmanager/static/js/home/models.js | 57 ++++-------------------
 remoteappmanager/webapi/application.py    | 15 +++---
 2 files changed, 17 insertions(+), 55 deletions(-)

diff --git a/remoteappmanager/static/js/home/models.js b/remoteappmanager/static/js/home/models.js
index ecad9a59c..2f258442e 100644
--- a/remoteappmanager/static/js/home/models.js
+++ b/remoteappmanager/static/js/home/models.js
@@ -25,54 +25,15 @@ define([
         var promise = $.Deferred();
 
         resources.Application.items()
-            .done(function (ids) {
-                // We neutralize the potential error from a jXHR request
-                // and make sure that all our requests "succeed" so that
-                // all/when can guarantee everything is done.
-
-                // These will go out of scope but they are still alive
-                // and performing to completion
-                var requests = [];
-                for (var i = 0; i < ids.length; i++) {
-                    requests.push($.Deferred());
-                }
-
-                // We need to bind with the current i index in the loop
-                // Hence we create these closures to grab the index for
-                // each new index
-                var done_callback = function(index) {
-                    return function(rep) {
-                        requests[index].resolve(rep);
-                    };
-                };
-                var fail_callback = function(index) {
-                    return function() {
-                        requests[index].resolve(null);
-                    };
-                };
-                
-                for (i = 0; i < ids.length; i++) {
-                    var id = ids[i];
-                    resources.Application.retrieve(id)
-                        .done(done_callback(i))
-                        .fail(fail_callback(i));
-
+            .done(function (items, offset, total) {
+                var result = [];
+                for (var key in items) {
+                    if (!items.hasOwnProperty(key)) {
+                        continue;
+                    }
+                    result.push(items[key]);
                 }
-
-                utils.all(requests)
-                    .done(function (promises) {
-                        // Fills the local application model with the results of the
-                        // retrieve promises.
-                        var data = [];
-                        for (var i = 0; i < promises.length; i++) {
-                            var result = promises[i];
-                            if (result !== null) {
-                                data.push(result);
-                            }
-                        }
-                        promise.resolve(data);
-                    });
-
+                promise.resolve(result);
             })
             .fail(function() {
                 promise.resolve([]);
@@ -163,7 +124,7 @@ define([
         var self = this;
         var app_data = self.app_data[index];
         
-        if (app_data.container === null) {
+        if (app_data.container === undefined) {
             self.status[index] = Status.STOPPED;
         } else {
             self.status[index] = Status.RUNNING;
diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index a40b1eee4..42c14e779 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -90,13 +90,14 @@ def items(self, items_response, **kwargs):
         container, if active, as values."""
         apps = self.application.db.get_apps_for_user(self.current_user.account)
 
-        container_manager = self.application.container_manager
-
         result = []
-        for mapping_id, app, policy in apps:
-            if (yield container_manager.image(app.image)) is not None:
-                resource = self.resource_class(identifier=mapping_id)
-
-                result.append(resource)
+        for mapping_id, _, _ in apps:
+            resource = self.resource_class(identifier=mapping_id)
+            try:
+                yield self.retrieve(resource)
+            except NotFound:
+                continue
+
+            result.append(resource)
 
         items_response.set(result)

From 375cb0fc839ba1f25dfc10f75675dc0ab1b67852 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 09:41:38 +0100
Subject: [PATCH 06/12] Updating application and container

---
 remoteappmanager/webapi/application.py        |  2 +
 remoteappmanager/webapi/container.py          |  2 +
 .../webapi/tests/test_application.py          | 52 ++++++++++++-------
 .../webapi/tests/test_container.py            |  5 +-
 4 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/remoteappmanager/webapi/application.py b/remoteappmanager/webapi/application.py
index 42c14e779..a006938d5 100644
--- a/remoteappmanager/webapi/application.py
+++ b/remoteappmanager/webapi/application.py
@@ -31,6 +31,7 @@ class Image(ResourceFragment):
 
 
 class Application(Resource):
+    mapping_id = Unicode(allow_empty=False, strip=True)
     image = OneOf(Image)
     container = OneOf(Container, optional=True)
 
@@ -66,6 +67,7 @@ def retrieve(self, resource, **kwargs):
             user_name=self.current_user.name,
             mapping_id=identifier)
 
+        resource.mapping_id = identifier
         resource.image = Image()
         resource.image.fill({
                 "name": image.name,
diff --git a/remoteappmanager/webapi/container.py b/remoteappmanager/webapi/container.py
index cd8a60d8d..35eb9be7b 100644
--- a/remoteappmanager/webapi/container.py
+++ b/remoteappmanager/webapi/container.py
@@ -51,8 +51,10 @@ def create(self, resource, **kwargs):
             raise exceptions.BadRepresentation(message="unrecognized image")
 
         try:
+            print("QQQ")
             environment = self._environment_from_configurables(image, resource)
         except Exception:
+            self.log.exception("Invalid configurables")
             raise exceptions.BadRepresentation(message="invalid configurables")
 
         # Everything is fine. Start and wait for the container to come online.
diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 9fc320469..647c6a9eb 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -61,14 +61,21 @@ def get_app(self):
 
     def test_items(self):
         _, data = self.get("/api/v1/applications/", httpstatus.OK)
-        self.assertEqual(data, {"items": ["one", "two"]})
+        self.assertEqual(data, {
+            "offset": 0,
+            "total": 2,
+            "items": [{}, {}]
+        })
 
         # Check if nothing is returned if no images are present
         self._app.container_manager.image = mock_coro_factory(
             return_value=None)
 
         _, data = self.get("/api/v1/applications/", httpstatus.OK)
-        self.assertEqual(data, {"items": []})
+        self.assertEqual(data, {
+            "offset": 0,
+            "total": 0,
+            "items": {}})
 
     def test_items_no_user(self):
         self.reg.authenticator = NullAuthenticator
@@ -78,7 +85,9 @@ def test_retrieve(self):
         _, data = self.get("/api/v1/applications/one/", httpstatus.OK)
 
         self.assertEqual(data,
-                         {'image': {
+                         {
+                             'mapping_id': "one",
+                             'image': {
                               'configurables': [],
                               'description': '',
                               'icon_128': '',
@@ -101,23 +110,26 @@ def test_retrieve(self):
         _, data = self.get("/api/v1/applications/one/", httpstatus.OK)
 
         self.assertEqual(data,
-                         {'container':
-                             {'image_name': 'xxx',
-                              'name': 'container',
-                              'url_id': 'yyy'
-                              },
-                          'image': {'description': '',
-                                    'icon_128': '',
-                                    'name': 'boo',
-                                    'ui_name': 'foo_ui',
-                                    'policy': {
-                                        "allow_home": True,
-                                        "volume_mode": 'ro',
-                                        "volume_source": "foo",
-                                        "volume_target": "bar",
-                                    },
-                                    'configurables': [],
-                                    }
+                         {
+                             'mapping_id': "one",
+                             'container': {
+                                 'image_name': 'xxx',
+                                 'name': 'container',
+                                 'url_id': 'yyy'
+                                 },
+                             'image': {
+                                 'description': '',
+                                 'icon_128': '',
+                                 'name': 'boo',
+                                 'ui_name': 'foo_ui',
+                                 'policy': {
+                                     "allow_home": True,
+                                     "volume_mode": 'ro',
+                                     "volume_source": "foo",
+                                     "volume_target": "bar",
+                                 },
+                                 'configurables': [],
+                             }
                           })
 
         self.get("/api/v1/applications/three/", httpstatus.NOT_FOUND)
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index 429d5481c..1189110b6 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -40,7 +40,10 @@ def test_items(self):
 
         # We get two because we have two mapping ids, hence the find_containers
         # gets called once per each mapping id.
-        self.assertEqual(data, {"items": ["12345", "12345"]})
+        self.assertEqual(data, {
+            "offset": 0,
+            "total": 2,
+            "items": ["12345", "12345"]})
 
     def test_create(self):
         with patch("remoteappmanager"

From 16d926967d2b0d45650d43bedc74e2f096540c35 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 11:25:47 +0100
Subject: [PATCH 07/12] Fixed tests

---
 .../webapi/tests/test_application.py          | 40 +++++++++++++++++--
 .../webapi/tests/test_container.py            | 19 +++++++--
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 647c6a9eb..79663c55d 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -62,9 +62,43 @@ def get_app(self):
     def test_items(self):
         _, data = self.get("/api/v1/applications/", httpstatus.OK)
         self.assertEqual(data, {
-            "offset": 0,
-            "total": 2,
-            "items": [{}, {}]
+            'total': 2,
+            'items': {
+                'two': {
+                    'image': {
+                        'policy': {
+                            'volume_mode': 'ro',
+                            'volume_source': 'foo',
+                            'allow_home': True,
+                            'volume_target': 'bar'
+                        },
+                        'name': 'boo',
+                        'icon_128': '',
+                        'ui_name': 'foo_ui',
+                        'description': '',
+                        'configurables': []
+                    },
+                    'mapping_id': 'two'
+                },
+                'one': {
+                    'image': {
+                        'policy': {
+                            'volume_mode': 'ro',
+                            'volume_source': 'foo',
+                            'allow_home': True,
+                            'volume_target': 'bar'
+                        },
+                        'name': 'boo',
+                        'icon_128': '',
+                        'ui_name': 'foo_ui',
+                        'description': '',
+                        'configurables': []
+                    },
+                    'mapping_id': 'one'
+                }
+            },
+            'identifiers': ['one', 'two'],
+            'offset': 0
         })
 
         # Check if nothing is returned if no images are present
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index 1189110b6..fbd24a7ec 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -40,10 +40,21 @@ def test_items(self):
 
         # We get two because we have two mapping ids, hence the find_containers
         # gets called once per each mapping id.
-        self.assertEqual(data, {
-            "offset": 0,
-            "total": 2,
-            "items": ["12345", "12345"]})
+        # This is a kind of unusual case, because we only get one item
+        # in the items list, due to the nature of the test.
+        self.assertEqual(
+            data,
+            {'identifiers': ['12345', '12345'],
+             'total': 2,
+             'offset': 0,
+             'items': {
+                '12345': {
+                    'image_name': 'image',
+                    'name': 'container',
+                    'mapping_id': 'whatever'
+                }
+            }
+        })
 
     def test_create(self):
         with patch("remoteappmanager"

From 71ae13e4d16618dc5dba4a01eabe9db874a914c3 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 11:58:19 +0100
Subject: [PATCH 08/12] Fixed remaining tests

---
 remoteappmanager/static/js/home/models.js         | 2 +-
 remoteappmanager/webapi/tests/test_application.py | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/remoteappmanager/static/js/home/models.js b/remoteappmanager/static/js/home/models.js
index 2f258442e..68980a4a2 100644
--- a/remoteappmanager/static/js/home/models.js
+++ b/remoteappmanager/static/js/home/models.js
@@ -25,7 +25,7 @@ define([
         var promise = $.Deferred();
 
         resources.Application.items()
-            .done(function (items, offset, total) {
+            .done(function (identifiers, items) {
                 var result = [];
                 for (var key in items) {
                     if (!items.hasOwnProperty(key)) {
diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 79663c55d..4e8b63b82 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -109,7 +109,8 @@ def test_items(self):
         self.assertEqual(data, {
             "offset": 0,
             "total": 0,
-            "items": {}})
+            "items": {},
+            "identifiers": []})
 
     def test_items_no_user(self):
         self.reg.authenticator = NullAuthenticator

From b70178b09a60c1d7474e562b38eebac7a5c44ef4 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 13:11:12 +0100
Subject: [PATCH 09/12] Fixed mock jsapi

---
 jstests/tests/home/mock_jsapi.js  | 73 +++++++++++++++----------------
 jstests/tests/home/test_models.js |  2 +-
 2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/jstests/tests/home/mock_jsapi.js b/jstests/tests/home/mock_jsapi.js
index eb18ad07b..1b0659334 100644
--- a/jstests/tests/home/mock_jsapi.js
+++ b/jstests/tests/home/mock_jsapi.js
@@ -3,51 +3,50 @@ define(['jquery'], function ($) {
 
     return {
         Application:  {
+            data: {
+                "12345": {
+                    image: {
+                        name: "app1",
+                        ui_name: "Application 1",
+                        icon_128: "",
+                        description: "description",
+                        policy: {
+                            allow_home: true,
+                            volume_source: "",
+                            volume_target: "",
+                            volume_mode: ""
+                        },
+                        configurables: [
+                            "resolution"
+                        ]
+                    }
+                },
+                "67890": {
+                    image: {
+                        name: "app2",
+                        ui_name: "Application 2",
+                        icon_128: "",
+                        description: "description",
+                        policy: {
+                            allow_home: true,
+                            volume_source: "",
+                            volume_target: "",
+                            volume_mode: ""
+                        },
+                        configurables: []
+                    }
+                }
+            },
             create: function() {
             },
             delete: function() {
             },
             retrieve: function(id) {
-                var data = {
-                    "12345": {
-                        image: {
-                            name: "app1",
-                            ui_name: "Application 1",
-                            icon_128: "",
-                            description: "description",
-                            policy: {
-                                allow_home: true,
-                                volume_source: "",
-                                volume_target: "",
-                                volume_mode: ""
-                            },
-                            configurables: [
-                                "resolution"
-                            ]
-                        }
-                    },
-                    "67890": {
-                        image: {
-                            name: "app2",
-                            ui_name: "Application 2",
-                            icon_128: "",
-                            description: "description",
-                            policy: {
-                                allow_home: true,
-                                volume_source: "",
-                                volume_target: "",
-                                volume_mode: ""
-                            },
-                            configurables: []
-                        }
-                    }
-                };
-                
-                return $.when(data[id]);
+                return $.when(this.data[id]);
                 
             },
             items: function() {
-                return $.when(["12345", "67890"]);
+                return $.when(["12345", "67890"], this.data);
             }
         },
         Container:  {
diff --git a/jstests/tests/home/test_models.js b/jstests/tests/home/test_models.js
index 4f53008b2..78a9c71ee 100644
--- a/jstests/tests/home/test_models.js
+++ b/jstests/tests/home/test_models.js
@@ -2,7 +2,7 @@ define([
     "home/models" 
 ], function (models) {
     "use strict";
-
+    
     QUnit.module("home.models");
     QUnit.test("instantiation", function (assert) {
         var model = new models.ApplicationListModel();

From cfa938665a2d01317ca19e37f833d2d56d47b8a4 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 13:20:19 +0100
Subject: [PATCH 10/12] flake

---
 remoteappmanager/webapi/tests/test_application.py | 3 +--
 remoteappmanager/webapi/tests/test_container.py   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/remoteappmanager/webapi/tests/test_application.py b/remoteappmanager/webapi/tests/test_application.py
index 4e8b63b82..c6fedb086 100644
--- a/remoteappmanager/webapi/tests/test_application.py
+++ b/remoteappmanager/webapi/tests/test_application.py
@@ -133,8 +133,7 @@ def test_retrieve(self):
                                     "volume_source": "foo",
                                     "volume_target": "bar",
                               },
-                              'ui_name': 'foo_ui',
-                          }})
+                              'ui_name': 'foo_ui'}})
 
         self._app.container_manager.find_containers = \
             mock_coro_factory(return_value=[Container(
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index fbd24a7ec..6fd98f39d 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -53,8 +53,7 @@ def test_items(self):
                     'name': 'container',
                     'mapping_id': 'whatever'
                 }
-            }
-        })
+            }})
 
     def test_create(self):
         with patch("remoteappmanager"

From 1342e1a94ea77bccfb31f270c67ba354fc944c81 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 24 Apr 2017 13:49:51 +0100
Subject: [PATCH 11/12] Rogue prints

---
 remoteappmanager/webapi/container.py            | 1 -
 remoteappmanager/webapi/tests/test_container.py | 1 -
 2 files changed, 2 deletions(-)

diff --git a/remoteappmanager/webapi/container.py b/remoteappmanager/webapi/container.py
index 35eb9be7b..87e8b9220 100644
--- a/remoteappmanager/webapi/container.py
+++ b/remoteappmanager/webapi/container.py
@@ -51,7 +51,6 @@ def create(self, resource, **kwargs):
             raise exceptions.BadRepresentation(message="unrecognized image")
 
         try:
-            print("QQQ")
             environment = self._environment_from_configurables(image, resource)
         except Exception:
             self.log.exception("Invalid configurables")
diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py
index 6fd98f39d..fecfa3644 100644
--- a/remoteappmanager/webapi/tests/test_container.py
+++ b/remoteappmanager/webapi/tests/test_container.py
@@ -213,7 +213,6 @@ def test_create_succeeds_for_empty_configurable(self):
                 httpstatus.CREATED
             )
 
-            print("YYY")
             self.post(
                 "/user/johndoe/api/v1/containers/",
                 dict(

From 86d697c732c6ba862b471cf11abf6ff8697ad59c Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Tue, 25 Apr 2017 15:05:59 +0100
Subject: [PATCH 12/12] Review

---
 remoteappmanager/static/js/home/models.js | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/remoteappmanager/static/js/home/models.js b/remoteappmanager/static/js/home/models.js
index 68980a4a2..cd363b2ff 100644
--- a/remoteappmanager/static/js/home/models.js
+++ b/remoteappmanager/static/js/home/models.js
@@ -27,12 +27,9 @@ define([
         resources.Application.items()
             .done(function (identifiers, items) {
                 var result = [];
-                for (var key in items) {
-                    if (!items.hasOwnProperty(key)) {
-                        continue;
-                    }
+                Object.keys(items).forEach(function(key) {
                     result.push(items[key]);
-                }
+                });
                 promise.resolve(result);
             })
             .fail(function() {