From 0889ea1818eb053ed901d5dd3e96550d81bfc2e3 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 17:07:25 +0100
Subject: [PATCH 01/25] Initial implementation

---
 remoteappmanager/rest/exceptions.py           | 27 ++++++++--
 remoteappmanager/rest/http/__init__.py        |  1 +
 .../rest/{ => http}/httpstatus.py             |  0
 .../rest/http/payloaded_http_error.py         | 15 ++++++
 remoteappmanager/rest/rest_handler.py         | 50 ++++++++++++++++---
 remoteappmanager/restresources/container.py   |  2 +-
 tests/rest/test_rest.py                       | 10 ++--
 tests/restmodel/test_application.py           |  8 ++-
 tests/restmodel/test_container.py             |  8 ++-
 9 files changed, 93 insertions(+), 28 deletions(-)
 create mode 100644 remoteappmanager/rest/http/__init__.py
 rename remoteappmanager/rest/{ => http}/httpstatus.py (100%)
 create mode 100644 remoteappmanager/rest/http/payloaded_http_error.py

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 4be58acb5..6af2a27fb 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -1,4 +1,4 @@
-from remoteappmanager.rest import httpstatus
+from remoteappmanager.rest.http import httpstatus
 
 
 class RESTException(Exception):
@@ -9,6 +9,23 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
+    def __init__(self, reason=None, **kwargs):
+        self.reason = reason
+        self.info = kwargs if len(kwargs) else None
+
+    def as_dict(self):
+        data = {
+            "type": type(self).__name__
+        }
+
+        if self.reason is not None:
+            data["reason"] = self.reason
+
+        if self.info is not None:
+            data["info"] = self.info
+
+        return data
+
 
 class NotFound(RESTException):
     """Exception raised when the resource is not found.
@@ -27,5 +44,9 @@ class BadRequest(RESTException):
     http_code = httpstatus.BAD_REQUEST
 
 
-class InternalServerError(RESTException):
-    pass
+class CannotCreate(RESTException):
+    """Exception raised when the resource cannot be created for
+    whatever reason.
+    """
+    http_code = httpstatus.INTERNAL_SERVER_ERROR
+
diff --git a/remoteappmanager/rest/http/__init__.py b/remoteappmanager/rest/http/__init__.py
new file mode 100644
index 000000000..8b1378917
--- /dev/null
+++ b/remoteappmanager/rest/http/__init__.py
@@ -0,0 +1 @@
+
diff --git a/remoteappmanager/rest/httpstatus.py b/remoteappmanager/rest/http/httpstatus.py
similarity index 100%
rename from remoteappmanager/rest/httpstatus.py
rename to remoteappmanager/rest/http/httpstatus.py
diff --git a/remoteappmanager/rest/http/payloaded_http_error.py b/remoteappmanager/rest/http/payloaded_http_error.py
new file mode 100644
index 000000000..9bbace570
--- /dev/null
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -0,0 +1,15 @@
+from tornado.httpclient import HTTPError
+
+
+class PayloadedHTTPError(HTTPError):
+    def __init__(self, code, payload, message=None, response=None):
+        """Provides a HTTPError that contains a string payload to output
+        as a response. If the payload is None, behaves like a regular
+        HTTPError, producing no payload in the response.
+        """
+        super().__init__(code, message, response)
+
+        if payload is not None and not isinstance(payload, str):
+            raise ValueError("payload must be a string.")
+
+        self.payload = payload
diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index 77a65aad9..8053e190f 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -1,9 +1,10 @@
 from remoteappmanager.handlers.base_handler import BaseHandler
-from tornado import gen, web, escape
-
+from remoteappmanager.rest import exceptions
+from remoteappmanager.rest.http import httpstatus
+from remoteappmanager.rest.http.payloaded_http_error import PayloadedHTTPError
 from remoteappmanager.rest.registry import registry
 from remoteappmanager.utils import url_path_join, with_end_slash
-from remoteappmanager.rest import httpstatus, exceptions
+from tornado import gen, web, escape
 
 
 class RESTBaseHandler(BaseHandler):
@@ -25,6 +26,37 @@ def get_resource_handler_or_404(self, collection_name):
         except KeyError:
             raise web.HTTPError(httpstatus.NOT_FOUND)
 
+    def write_error(self, status_code, **kwargs):
+        """Provides appropriate payload to the response in case of error.
+        """
+        exc_info = kwargs.get("exc_info")
+
+        if exc_info is None:
+            self.finish()
+
+        exc = exc_info[1]
+        if isinstance(exc, PayloadedHTTPError):
+            self.finish(exc.payload)
+        else:
+            # For non-payloaded http errors or any other exception
+            # we don't want to return anything as payload.
+            # The error code is enough.
+            self.finish()
+
+    def rest_to_http_exception(self, rest_exc):
+        """Converts a REST exception into the appropriate HTTP one."""
+        if isinstance(rest_exc, exceptions.NotFound):
+            # NotFound is a special case, because it should have no payload,
+            # just the http 404
+            return web.HTTPError(code=httpstatus.NOT_FOUND)
+
+        return PayloadedHTTPError(
+            code=rest_exc.http_code,
+            payload=escape.json_encode({
+                "error": rest_exc.as_dict()
+            })
+        )
+
 
 class RESTCollectionHandler(RESTBaseHandler):
     """Handler for URLs addressing a collection.
@@ -38,7 +70,7 @@ def get(self, collection_name):
         try:
             items = yield res_handler.items()
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -67,7 +99,7 @@ def post(self, collection_name):
         try:
             resource_id = yield res_handler.create(data)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -104,7 +136,7 @@ def get(self, collection_name, identifier):
         try:
             representation = yield res_handler.retrieve(identifier)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -128,6 +160,8 @@ def post(self, collection_name, identifier):
 
         try:
             exists = yield res_handler.exists(identifier)
+        except exceptions.RESTException as e:
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -156,7 +190,7 @@ def put(self, collection_name, identifier):
         try:
             yield res_handler.update(identifier, representation)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -176,7 +210,7 @@ def delete(self, collection_name, identifier):
         try:
             yield res_handler.delete(identifier)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 5ad389e67..c42a4c326 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -54,7 +54,7 @@ def create(self, representation):
                         "container".format(
                             container.docker_id))
 
-            raise exceptions.InternalServerError()
+            raise exceptions.CannotCreate()
 
         urlpath = url_path_join(
             self.application.command_line_config.base_urlpath,
diff --git a/tests/rest/test_rest.py b/tests/rest/test_rest.py
index db15d58a8..24c6bf654 100644
--- a/tests/rest/test_rest.py
+++ b/tests/rest/test_rest.py
@@ -1,18 +1,16 @@
 import unittest
 import urllib.parse
-from unittest import mock
-
-from tests import utils
-from tornado import web, gen, escape
 from collections import OrderedDict
+from unittest import mock
 
 from remoteappmanager import rest
+from remoteappmanager.rest import registry, httpstatus, exceptions
 from remoteappmanager.rest.resource import Resource
 from remoteappmanager.rest.rest_handler import RESTResourceHandler, \
     RESTCollectionHandler
-from remoteappmanager.rest import registry, httpstatus, exceptions
-
+from tests import utils
 from tests.utils import AsyncHTTPTestCase
+from tornado import web, gen, escape
 
 
 def prepare_side_effect(*args, **kwargs):
diff --git a/tests/restmodel/test_application.py b/tests/restmodel/test_application.py
index a08a81627..89af64ded 100644
--- a/tests/restmodel/test_application.py
+++ b/tests/restmodel/test_application.py
@@ -1,13 +1,11 @@
 from unittest.mock import Mock, patch
 
-from remoteappmanager.restresources import Application
-from tests import utils
-from tornado import web, escape
-
 from remoteappmanager import rest
 from remoteappmanager.rest import registry, httpstatus
-
+from remoteappmanager.restresources import Application
+from tests import utils
 from tests.utils import AsyncHTTPTestCase
+from tornado import web, escape
 
 
 class TestApplication(AsyncHTTPTestCase):
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index fd6c89af4..d7e4b72a2 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,14 +1,12 @@
 from unittest.mock import Mock, patch
 
-from remoteappmanager.restresources import Container
-from tornado import web, escape
-
 from remoteappmanager import rest
 from remoteappmanager.rest import registry, httpstatus
-
+from remoteappmanager.restresources import Container
+from tests.mocking import dummy
 from tests.utils import (AsyncHTTPTestCase, mock_coro_factory,
                          mock_coro_new_callable, containers_dict)
-from tests.mocking import dummy
+from tornado import web, escape
 
 
 class TestContainer(AsyncHTTPTestCase):

From 976479716b17056b2d3e65cdbcdcd6c327086ea8 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 17:40:24 +0100
Subject: [PATCH 02/25] Added more control, and generalized exception

---
 remoteappmanager/rest/exceptions.py         |  7 +-
 remoteappmanager/restresources/container.py | 82 +++++++++++++++------
 2 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 6af2a27fb..debe1e3f3 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -44,9 +44,8 @@ class BadRequest(RESTException):
     http_code = httpstatus.BAD_REQUEST
 
 
-class CannotCreate(RESTException):
-    """Exception raised when the resource cannot be created for
-    whatever reason.
+class Unable(RESTException):
+    """Exception raised when the CRUD request cannot be performed
+    for whatever reason that is not dependent on the client.
     """
     http_code = httpstatus.INTERNAL_SERVER_ERROR
-
diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index c42a4c326..35a1a6597 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -1,3 +1,4 @@
+import contextlib
 import os
 from datetime import timedelta
 
@@ -17,11 +18,13 @@ def create(self, representation):
         """Create the container.
         The representation should accept the application mapping id we
         want to start"""
-        mapping_id = representation["mapping_id"]
+        try:
+            mapping_id = representation["mapping_id"]
+        except KeyError:
+            raise exceptions.BadRequest(reason="missing mapping_id")
 
         account = self.current_user.account
         all_apps = self.application.db.get_apps_for_user(account)
-        container_manager = self.application.container_manager
 
         choice = [(m_id, app, policy)
                   for m_id, app, policy in all_apps
@@ -30,10 +33,9 @@ def create(self, representation):
         if not choice:
             self.log.warning("Could not find resource "
                              "for mapping id {}".format(mapping_id))
-            raise exceptions.BadRequest()
+            raise exceptions.BadRequest(reason="unrecognized mapping_id")
 
         _, app, policy = choice[0]
-        container = None
 
         try:
             container = yield self._start_container(
@@ -41,26 +43,25 @@ def create(self, representation):
                 app,
                 policy,
                 mapping_id)
-            yield self._wait_for_container_ready(container)
         except Exception as e:
-            if container is not None:
-                try:
-                    yield container_manager.stop_and_remove_container(
-                        container.docker_id)
-                except Exception:
-                    self.log.exception(
-                        "Unable to stop container {} after "
-                        " failure to obtain a ready "
-                        "container".format(
-                            container.docker_id))
-
-            raise exceptions.CannotCreate()
+            raise exceptions.Unable(reason=str(e))
+
+        try:
+            with self._remove_container_on_error(container):
+                self._wait_for_container_ready(container)
+        except Exception as e:
+            raise exceptions.Unable(reason=str(e))
 
         urlpath = url_path_join(
             self.application.command_line_config.base_urlpath,
             container.urlpath)
-        yield self.application.reverse_proxy.register(urlpath,
-                                                      container.host_url)
+
+        try:
+            with self._remove_container_on_error(container):
+                yield self.application.reverse_proxy.register(
+                    urlpath, container.host_url)
+        except Exception as e:
+            raise exceptions.Unable(reason=str(e))
 
         return container.url_id
 
@@ -83,6 +84,8 @@ def retrieve(self, identifier):
     def delete(self, identifier):
         """Stop the container."""
         container = yield self._container_from_url_id(identifier)
+        container_manager = self.application.container_manager
+
         if not container:
             self.log.warning("Could not find container for id {}".format(
                              identifier))
@@ -91,9 +94,22 @@ def delete(self, identifier):
         urlpath = url_path_join(
             self.application.command_line_config.base_urlpath,
             container.urlpath)
-        yield self.application.reverse_proxy.unregister(urlpath)
-        yield self.application.container_manager.stop_and_remove_container(
-            container.docker_id)
+
+        try:
+            yield self.application.reverse_proxy.unregister(urlpath)
+        except Exception:
+            # 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))
+
+        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))
 
     @gen.coroutine
     def items(self):
@@ -125,6 +141,28 @@ def items(self):
 
         return running_containers
 
+    ##################
+    # Private
+
+    @contextlib.contextmanager
+    def _remove_container_on_error(self, container):
+        """Context manager that guarantees we remove the container
+        if something goes wrong during the context-held operation"""
+        container_manager = self.application.container_manager
+        try:
+            yield
+        except Exception as e:
+            try:
+                yield container_manager.stop_and_remove_container(
+                    container.docker_id)
+            except Exception:
+                self.log.exception(
+                    "Unable to stop container {} after "
+                    " failure to obtain a ready "
+                    "container".format(
+                        container.docker_id))
+            raise e
+
     @gen.coroutine
     def _container_from_url_id(self, container_url_id):
         """Retrieves and returns the container if valid and present.

From eeda70b8ad8a4bd605ccbe5bebf33659e8325840 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 18:04:57 +0100
Subject: [PATCH 03/25] Wrong HTTPError

---
 remoteappmanager/rest/http/__init__.py             | 1 -
 remoteappmanager/rest/http/payloaded_http_error.py | 6 +++---
 remoteappmanager/rest/rest_handler.py              | 4 ++--
 tests/rest/test_rest.py                            | 5 +++--
 tests/restmodel/test_application.py                | 3 ++-
 tests/restmodel/test_container.py                  | 3 ++-
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/remoteappmanager/rest/http/__init__.py b/remoteappmanager/rest/http/__init__.py
index 8b1378917..e69de29bb 100644
--- a/remoteappmanager/rest/http/__init__.py
+++ b/remoteappmanager/rest/http/__init__.py
@@ -1 +0,0 @@
-
diff --git a/remoteappmanager/rest/http/payloaded_http_error.py b/remoteappmanager/rest/http/payloaded_http_error.py
index 9bbace570..c54c10979 100644
--- a/remoteappmanager/rest/http/payloaded_http_error.py
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -1,13 +1,13 @@
-from tornado.httpclient import HTTPError
+from tornado.web import HTTPError
 
 
 class PayloadedHTTPError(HTTPError):
-    def __init__(self, code, payload, message=None, response=None):
+    def __init__(self, status_code, payload, log_message=None, *args, **kwargs):
         """Provides a HTTPError that contains a string payload to output
         as a response. If the payload is None, behaves like a regular
         HTTPError, producing no payload in the response.
         """
-        super().__init__(code, message, response)
+        super().__init__(status_code, log_message, *args, **kwargs)
 
         if payload is not None and not isinstance(payload, str):
             raise ValueError("payload must be a string.")
diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index 8053e190f..fdb0eab78 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -48,10 +48,10 @@ def rest_to_http_exception(self, rest_exc):
         if isinstance(rest_exc, exceptions.NotFound):
             # NotFound is a special case, because it should have no payload,
             # just the http 404
-            return web.HTTPError(code=httpstatus.NOT_FOUND)
+            return web.HTTPError(status_code=httpstatus.NOT_FOUND)
 
         return PayloadedHTTPError(
-            code=rest_exc.http_code,
+            status_code=rest_exc.http_code,
             payload=escape.json_encode({
                 "error": rest_exc.as_dict()
             })
diff --git a/tests/rest/test_rest.py b/tests/rest/test_rest.py
index 24c6bf654..54dcfd6a3 100644
--- a/tests/rest/test_rest.py
+++ b/tests/rest/test_rest.py
@@ -4,7 +4,8 @@
 from unittest import mock
 
 from remoteappmanager import rest
-from remoteappmanager.rest import registry, httpstatus, exceptions
+from remoteappmanager.rest import registry, exceptions
+from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.rest.resource import Resource
 from remoteappmanager.rest.rest_handler import RESTResourceHandler, \
     RESTCollectionHandler
@@ -48,7 +49,7 @@ def update(self, identifier, representation):
     @gen.coroutine
     def delete(self, identifier):
         if identifier not in self.collection:
-            raise exceptions.NotFound
+            raise exceptions.NotFound()
 
         del self.collection[identifier]
 
diff --git a/tests/restmodel/test_application.py b/tests/restmodel/test_application.py
index 89af64ded..a4fd9de14 100644
--- a/tests/restmodel/test_application.py
+++ b/tests/restmodel/test_application.py
@@ -1,7 +1,8 @@
 from unittest.mock import Mock, patch
 
 from remoteappmanager import rest
-from remoteappmanager.rest import registry, httpstatus
+from remoteappmanager.rest import registry
+from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.restresources import Application
 from tests import utils
 from tests.utils import AsyncHTTPTestCase
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index d7e4b72a2..ad3c05dd6 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,7 +1,8 @@
 from unittest.mock import Mock, patch
 
 from remoteappmanager import rest
-from remoteappmanager.rest import registry, httpstatus
+from remoteappmanager.rest import registry
+from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.restresources import Container
 from tests.mocking import dummy
 from tests.utils import (AsyncHTTPTestCase, mock_coro_factory,

From 25caf306b30fa6e6d25d8ae219668b3333aee652 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 20:11:13 +0100
Subject: [PATCH 04/25] Fixed documentation

---
 doc/Makefile                                  |  1 +
 doc/source/api/remoteappmanager.db.rst        |  4 ---
 doc/source/api/remoteappmanager.rest.http.rst | 30 +++++++++++++++++++
 doc/source/api/remoteappmanager.rest.rst      | 15 +++++-----
 doc/source/api/remoteappmanager.rst           |  8 +++++
 5 files changed, 46 insertions(+), 12 deletions(-)
 create mode 100644 doc/source/api/remoteappmanager.rest.http.rst

diff --git a/doc/Makefile b/doc/Makefile
index 580483dbf..22a433c81 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -44,6 +44,7 @@ help:
 	@echo "  coverage   to run coverage check of the documentation (if enabled)"
 	@echo "  dummy      to check syntax errors of document sources"
 	@echo "  uml        to convert all uml files into images"
+	@echo "  apidoc     to rebuild the apidoc documents"
 
 .PHONY: clean
 clean:
diff --git a/doc/source/api/remoteappmanager.db.rst b/doc/source/api/remoteappmanager.db.rst
index e33719b6c..411898e03 100644
--- a/doc/source/api/remoteappmanager.db.rst
+++ b/doc/source/api/remoteappmanager.db.rst
@@ -9,7 +9,6 @@ remoteappmanager.db.csv_db module
 
 .. automodule:: remoteappmanager.db.csv_db
     :members:
-    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -18,7 +17,6 @@ remoteappmanager.db.interfaces module
 
 .. automodule:: remoteappmanager.db.interfaces
     :members:
-    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -27,7 +25,6 @@ remoteappmanager.db.orm module
 
 .. automodule:: remoteappmanager.db.orm
     :members:
-    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -37,6 +34,5 @@ Module contents
 
 .. automodule:: remoteappmanager.db
     :members:
-    :special-members: __init__
     :undoc-members:
     :show-inheritance:
diff --git a/doc/source/api/remoteappmanager.rest.http.rst b/doc/source/api/remoteappmanager.rest.http.rst
new file mode 100644
index 000000000..d045864d9
--- /dev/null
+++ b/doc/source/api/remoteappmanager.rest.http.rst
@@ -0,0 +1,30 @@
+remoteappmanager.rest.http package
+==================================
+
+Submodules
+----------
+
+remoteappmanager.rest.http.httpstatus module
+--------------------------------------------
+
+.. automodule:: remoteappmanager.rest.http.httpstatus
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+remoteappmanager.rest.http.payloaded_http_error module
+------------------------------------------------------
+
+.. automodule:: remoteappmanager.rest.http.payloaded_http_error
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+
+Module contents
+---------------
+
+.. automodule:: remoteappmanager.rest.http
+    :members:
+    :undoc-members:
+    :show-inheritance:
diff --git a/doc/source/api/remoteappmanager.rest.rst b/doc/source/api/remoteappmanager.rest.rst
index a84fc0292..5615c69a1 100644
--- a/doc/source/api/remoteappmanager.rest.rst
+++ b/doc/source/api/remoteappmanager.rest.rst
@@ -1,6 +1,13 @@
 remoteappmanager.rest package
 =============================
 
+Subpackages
+-----------
+
+.. toctree::
+
+    remoteappmanager.rest.http
+
 Submodules
 ----------
 
@@ -12,14 +19,6 @@ remoteappmanager.rest.exceptions module
     :undoc-members:
     :show-inheritance:
 
-remoteappmanager.rest.httpstatus module
----------------------------------------
-
-.. automodule:: remoteappmanager.rest.httpstatus
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
 remoteappmanager.rest.registry module
 -------------------------------------
 
diff --git a/doc/source/api/remoteappmanager.rst b/doc/source/api/remoteappmanager.rst
index 84edd771f..8db8b7de2 100644
--- a/doc/source/api/remoteappmanager.rst
+++ b/doc/source/api/remoteappmanager.rst
@@ -58,6 +58,14 @@ remoteappmanager.jinja2_adapters module
     :undoc-members:
     :show-inheritance:
 
+remoteappmanager.netutils module
+--------------------------------
+
+.. automodule:: remoteappmanager.netutils
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 remoteappmanager.paths module
 -----------------------------
 

From dabad45c48f5bb5f83d24791485d25028fd9027c Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 20:11:21 +0100
Subject: [PATCH 05/25] Fixed flake

---
 remoteappmanager/rest/exceptions.py                | 1 +
 remoteappmanager/rest/http/payloaded_http_error.py | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index debe1e3f3..6f179e29c 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -14,6 +14,7 @@ def __init__(self, reason=None, **kwargs):
         self.info = kwargs if len(kwargs) else None
 
     def as_dict(self):
+        """Returns a dictionary with the details of the exception"""
         data = {
             "type": type(self).__name__
         }
diff --git a/remoteappmanager/rest/http/payloaded_http_error.py b/remoteappmanager/rest/http/payloaded_http_error.py
index c54c10979..51f6453ab 100644
--- a/remoteappmanager/rest/http/payloaded_http_error.py
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -2,7 +2,8 @@
 
 
 class PayloadedHTTPError(HTTPError):
-    def __init__(self, status_code, payload, log_message=None, *args, **kwargs):
+    def __init__(self, status_code, payload=None, log_message=None,
+                 *args, **kwargs):
         """Provides a HTTPError that contains a string payload to output
         as a response. If the payload is None, behaves like a regular
         HTTPError, producing no payload in the response.

From fb137acf6af3b85d2715095bdacfe552dc1ddc70 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 20:59:54 +0100
Subject: [PATCH 06/25] Added missing yield

---
 remoteappmanager/restresources/container.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 35a1a6597..c95a6dd56 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -48,7 +48,7 @@ def create(self, representation):
 
         try:
             with self._remove_container_on_error(container):
-                self._wait_for_container_ready(container)
+                yield self._wait_for_container_ready(container)
         except Exception as e:
             raise exceptions.Unable(reason=str(e))
 

From 8b5909b84b89b25a8270916d2013f4ffbcb0a0f6 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 21:23:06 +0100
Subject: [PATCH 07/25] Adapted against current json handler in javascript

---
 remoteappmanager/rest/exceptions.py         | 10 +++++-----
 remoteappmanager/rest/rest_handler.py       |  4 +---
 remoteappmanager/restresources/container.py | 10 +++++-----
 remoteappmanager/static/js/remoteappapi.js  |  1 -
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 6f179e29c..74b19541c 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -9,8 +9,8 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
-    def __init__(self, reason=None, **kwargs):
-        self.reason = reason
+    def __init__(self, message=None, **kwargs):
+        self.message = message
         self.info = kwargs if len(kwargs) else None
 
     def as_dict(self):
@@ -19,11 +19,11 @@ def as_dict(self):
             "type": type(self).__name__
         }
 
-        if self.reason is not None:
-            data["reason"] = self.reason
+        if self.message is not None:
+            data["message"] = self.message
 
         if self.info is not None:
-            data["info"] = self.info
+            data.update(self.info)
 
         return data
 
diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index fdb0eab78..d7dc18e87 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -52,9 +52,7 @@ def rest_to_http_exception(self, rest_exc):
 
         return PayloadedHTTPError(
             status_code=rest_exc.http_code,
-            payload=escape.json_encode({
-                "error": rest_exc.as_dict()
-            })
+            payload=escape.json_encode(rest_exc.as_dict())
         )
 
 
diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index c95a6dd56..dc7d289e0 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -21,7 +21,7 @@ def create(self, representation):
         try:
             mapping_id = representation["mapping_id"]
         except KeyError:
-            raise exceptions.BadRequest(reason="missing mapping_id")
+            raise exceptions.BadRequest(message="missing mapping_id")
 
         account = self.current_user.account
         all_apps = self.application.db.get_apps_for_user(account)
@@ -33,7 +33,7 @@ def create(self, representation):
         if not choice:
             self.log.warning("Could not find resource "
                              "for mapping id {}".format(mapping_id))
-            raise exceptions.BadRequest(reason="unrecognized mapping_id")
+            raise exceptions.BadRequest(message="unrecognized mapping_id")
 
         _, app, policy = choice[0]
 
@@ -44,13 +44,13 @@ def create(self, representation):
                 policy,
                 mapping_id)
         except Exception as e:
-            raise exceptions.Unable(reason=str(e))
+            raise exceptions.Unable(message=str(e))
 
         try:
             with self._remove_container_on_error(container):
                 yield self._wait_for_container_ready(container)
         except Exception as e:
-            raise exceptions.Unable(reason=str(e))
+            raise exceptions.Unable(message=str(e))
 
         urlpath = url_path_join(
             self.application.command_line_config.base_urlpath,
@@ -61,7 +61,7 @@ def create(self, representation):
                 yield self.application.reverse_proxy.register(
                     urlpath, container.host_url)
         except Exception as e:
-            raise exceptions.Unable(reason=str(e))
+            raise exceptions.Unable(message=str(e))
 
         return container.url_id
 
diff --git a/remoteappmanager/static/js/remoteappapi.js b/remoteappmanager/static/js/remoteappapi.js
index 3de176ad6..7d680f1f5 100644
--- a/remoteappmanager/static/js/remoteappapi.js
+++ b/remoteappmanager/static/js/remoteappapi.js
@@ -46,7 +46,6 @@ define(['jquery', 'utils'], function ($, utils) {
         options = options || {};
         options = update(options, {
             type: 'POST', 
-            dataType: null,
             data: JSON.stringify({
                 mapping_id: id
             })});

From 873af13cffb17e8e446c8392f25ece88b799d36e Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 22:24:00 +0100
Subject: [PATCH 08/25] Polished the concept of representable exceptions

---
 remoteappmanager/rest/exceptions.py   | 18 +++++++++++-------
 remoteappmanager/rest/rest_handler.py | 11 ++++++-----
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 74b19541c..351346be7 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -9,19 +9,18 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
-    def __init__(self, message=None, **kwargs):
-        self.message = message
+    def __init__(self, **kwargs):
+        """Initializes the exception. keyword arguments will become
+        part of the representation as key/value pairs."""
         self.info = kwargs if len(kwargs) else None
 
-    def as_dict(self):
-        """Returns a dictionary with the details of the exception"""
+    def representation(self):
+        """Returns a dictionary with the representation of the exception.
+        """
         data = {
             "type": type(self).__name__
         }
 
-        if self.message is not None:
-            data["message"] = self.message
-
         if self.info is not None:
             data.update(self.info)
 
@@ -35,6 +34,11 @@ class NotFound(RESTException):
     """
     http_code = httpstatus.NOT_FOUND
 
+    def representation(self):
+        """NotFound is special as it does not have a representation,
+        just an error status"""
+        return None
+
 
 class BadRequest(RESTException):
     """Exception raised when the resource representation is
diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index d7dc18e87..c47769ec8 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -45,14 +45,15 @@ def write_error(self, status_code, **kwargs):
 
     def rest_to_http_exception(self, rest_exc):
         """Converts a REST exception into the appropriate HTTP one."""
-        if isinstance(rest_exc, exceptions.NotFound):
-            # NotFound is a special case, because it should have no payload,
-            # just the http 404
-            return web.HTTPError(status_code=httpstatus.NOT_FOUND)
+
+        representation = rest_exc.representation()
+        payload = (escape.json_encode(representation)
+                   if representation is not None
+                   else None)
 
         return PayloadedHTTPError(
             status_code=rest_exc.http_code,
-            payload=escape.json_encode(rest_exc.as_dict())
+            payload=payload
         )
 
 

From 35f42e4f16e3d1a6ab226af09f727d15216c61ab Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Wed, 27 Jul 2016 23:08:34 +0100
Subject: [PATCH 09/25] Workaround for 201 Created error and Content-Type
 correctness

---
 remoteappmanager/rest/rest_handler.py      | 5 +++++
 remoteappmanager/static/js/remoteappapi.js | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index c47769ec8..b6c31d462 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -36,11 +36,13 @@ def write_error(self, status_code, **kwargs):
 
         exc = exc_info[1]
         if isinstance(exc, PayloadedHTTPError):
+            self.set_header('Content-Type', 'application/json')
             self.finish(exc.payload)
         else:
             # For non-payloaded http errors or any other exception
             # we don't want to return anything as payload.
             # The error code is enough.
+            self.clear_header('Content-Type')
             self.finish()
 
     def rest_to_http_exception(self, rest_exc):
@@ -118,6 +120,7 @@ def post(self, collection_name):
 
         self.set_status(httpstatus.CREATED)
         self.set_header("Location", location)
+        self.clear_header('Content-Type')
         self.flush()
 
 
@@ -199,6 +202,7 @@ def put(self, collection_name, identifier):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)
 
     @web.authenticated
@@ -219,4 +223,5 @@ def delete(self, collection_name, identifier):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)
diff --git a/remoteappmanager/static/js/remoteappapi.js b/remoteappmanager/static/js/remoteappapi.js
index 7d680f1f5..f7fdb514b 100644
--- a/remoteappmanager/static/js/remoteappapi.js
+++ b/remoteappmanager/static/js/remoteappapi.js
@@ -9,7 +9,7 @@ define(['jquery', 'utils'], function ($, utils) {
         type: 'GET',
         contentType: "application/json",
         cache: false,
-        dataType : "json",
+        dataType : null, 
         processData: false,
         success: null,
         error: null
@@ -57,7 +57,7 @@ define(['jquery', 'utils'], function ($, utils) {
 
     RemoteAppAPI.prototype.stop_application = function (id, options) {
         options = options || {};
-        options = update(options, {type: 'DELETE', dataType: null});
+        options = update(options, {type: 'DELETE'});
         this.api_request(
             utils.url_path_join('containers', id),
             options

From 424e572c00090092ad4161891c8bc167d9c861d3 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 09:17:45 +0100
Subject: [PATCH 10/25] Better to have the message explicit.

---
 remoteappmanager/rest/exceptions.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 351346be7..6ccc584e6 100644
--- a/remoteappmanager/rest/exceptions.py
+++ b/remoteappmanager/rest/exceptions.py
@@ -9,9 +9,10 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
-    def __init__(self, **kwargs):
+    def __init__(self, message=None, **kwargs):
         """Initializes the exception. keyword arguments will become
         part of the representation as key/value pairs."""
+        self.message = message
         self.info = kwargs if len(kwargs) else None
 
     def representation(self):
@@ -20,6 +21,8 @@ def representation(self):
         data = {
             "type": type(self).__name__
         }
+        if self.message is not None:
+            data["message"] = self.message
 
         if self.info is not None:
             data.update(self.info)

From 138f8fd0baebb7c6367ef16595c93954a1ca0526 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 09:27:51 +0100
Subject: [PATCH 11/25] Set content type on the payloaded exception

---
 .../rest/http/payloaded_http_error.py         | 23 ++++++++++++++++---
 remoteappmanager/rest/rest_handler.py         |  8 ++++---
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/remoteappmanager/rest/http/payloaded_http_error.py b/remoteappmanager/rest/http/payloaded_http_error.py
index 51f6453ab..4c47b9541 100644
--- a/remoteappmanager/rest/http/payloaded_http_error.py
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -2,15 +2,32 @@
 
 
 class PayloadedHTTPError(HTTPError):
-    def __init__(self, status_code, payload=None, log_message=None,
+    def __init__(self, status_code,
+                 payload=None,
+                 content_type=None,
+                 log_message=None,
                  *args, **kwargs):
         """Provides a HTTPError that contains a string payload to output
         as a response. If the payload is None, behaves like a regular
         HTTPError, producing no payload in the response.
+
+        Parameters
+        ----------
+        payload: str or None
+            The payload as a string
+        content_type: str or None
+            The content type of the payload
+        log_message: str or None
+            The log message. Passed to the HTTPError.
         """
         super().__init__(status_code, log_message, *args, **kwargs)
 
-        if payload is not None and not isinstance(payload, str):
-            raise ValueError("payload must be a string.")
+        if payload is not None:
+            if not isinstance(payload, str):
+                raise ValueError("payload must be a string.")
+
+            if content_type is None:
+                content_type = "text/plain"
 
+        self.content_type = content_type
         self.payload = payload
diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index b6c31d462..0326d16d1 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -35,8 +35,9 @@ def write_error(self, status_code, **kwargs):
             self.finish()
 
         exc = exc_info[1]
-        if isinstance(exc, PayloadedHTTPError):
-            self.set_header('Content-Type', 'application/json')
+
+        if isinstance(exc, PayloadedHTTPError) and exc.payload is not None:
+            self.set_header('Content-Type', exc.content_type)
             self.finish(exc.payload)
         else:
             # For non-payloaded http errors or any other exception
@@ -55,7 +56,8 @@ def rest_to_http_exception(self, rest_exc):
 
         return PayloadedHTTPError(
             status_code=rest_exc.http_code,
-            payload=payload
+            payload=payload,
+            content_type="application/json"
         )
 
 

From cc26710962b891ebe0af75b11059831707e4d246 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 10:35:25 +0100
Subject: [PATCH 12/25] Test for payloaded http error

---
 .../rest/http/payloaded_http_error.py         |  3 +++
 tests/rest/http/__init__.py                   |  0
 tests/rest/http/test_payloaded_http_error.py  | 27 +++++++++++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 tests/rest/http/__init__.py
 create mode 100644 tests/rest/http/test_payloaded_http_error.py

diff --git a/remoteappmanager/rest/http/payloaded_http_error.py b/remoteappmanager/rest/http/payloaded_http_error.py
index 4c47b9541..dd37b1ed0 100644
--- a/remoteappmanager/rest/http/payloaded_http_error.py
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -28,6 +28,9 @@ def __init__(self, status_code,
 
             if content_type is None:
                 content_type = "text/plain"
+        else:
+            if content_type is not None:
+                raise ValueError("Content type specified, but no payload")
 
         self.content_type = content_type
         self.payload = payload
diff --git a/tests/rest/http/__init__.py b/tests/rest/http/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/rest/http/test_payloaded_http_error.py b/tests/rest/http/test_payloaded_http_error.py
new file mode 100644
index 000000000..94e4dfa1d
--- /dev/null
+++ b/tests/rest/http/test_payloaded_http_error.py
@@ -0,0 +1,27 @@
+import unittest
+
+from remoteappmanager.rest.http.payloaded_http_error import PayloadedHTTPError
+
+
+class TestPayloadedHTTPError(unittest.TestCase):
+    def test_init(self):
+        payloaded = PayloadedHTTPError(500, payload=None)
+        self.assertEqual(payloaded.payload, None)
+        self.assertEqual(payloaded.content_type, None)
+
+        with self.assertRaises(ValueError):
+            PayloadedHTTPError(500, payload=123)
+
+        with self.assertRaises(ValueError):
+            PayloadedHTTPError(500, content_type="text/plain")
+
+        payloaded = PayloadedHTTPError(500,
+                                       payload="hello",
+                                       content_type="text/html")
+
+        self.assertEqual(payloaded.payload, "hello")
+        self.assertEqual(payloaded.content_type, "text/html")
+
+        payloaded = PayloadedHTTPError(500, payload="hello")
+        self.assertEqual(payloaded.content_type, "text/plain")
+        self.assertEqual(payloaded.status_code, 500)

From 61b10f4a3c125c36a60ee047216ffecb96666c82 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 10:36:19 +0100
Subject: [PATCH 13/25] Removed content type when finish on empty exception

---
 remoteappmanager/rest/rest_handler.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index 0326d16d1..e170de36c 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -32,6 +32,7 @@ def write_error(self, status_code, **kwargs):
         exc_info = kwargs.get("exc_info")
 
         if exc_info is None:
+            self.clear_header('Content-Type')
             self.finish()
 
         exc = exc_info[1]

From ba04eafceab8be2cfb92ae78e62b23b6036f8e52 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 11:05:23 +0100
Subject: [PATCH 14/25] Added more tests

---
 remoteappmanager/rest/rest_handler.py | 11 ++++---
 tests/rest/test_rest.py               | 46 +++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index e170de36c..78a97c972 100644
--- a/remoteappmanager/rest/rest_handler.py
+++ b/remoteappmanager/rest/rest_handler.py
@@ -51,14 +51,17 @@ def rest_to_http_exception(self, rest_exc):
         """Converts a REST exception into the appropriate HTTP one."""
 
         representation = rest_exc.representation()
-        payload = (escape.json_encode(representation)
-                   if representation is not None
-                   else None)
+        payload = None
+        content_type = None
+
+        if representation is not None:
+            payload = escape.json_encode(representation)
+            content_type = "application/json"
 
         return PayloadedHTTPError(
             status_code=rest_exc.http_code,
             payload=payload,
-            content_type="application/json"
+            content_type=content_type
         )
 
 
diff --git a/tests/rest/test_rest.py b/tests/rest/test_rest.py
index 54dcfd6a3..ea5e3c5c4 100644
--- a/tests/rest/test_rest.py
+++ b/tests/rest/test_rest.py
@@ -65,11 +65,15 @@ class UnsupportAll(Resource):
 class Unprocessable(Resource):
     @gen.coroutine
     def create(self, representation):
-        raise exceptions.BadRequest()
+        raise exceptions.BadRequest("unprocessable", foo="bar")
 
     @gen.coroutine
     def update(self, identifier, representation):
-        raise exceptions.BadRequest()
+        raise exceptions.BadRequest("unprocessable", foo="bar")
+
+    @gen.coroutine
+    def retrieve(self, identifier):
+        raise exceptions.BadRequest("unprocessable", foo="bar")
 
 
 class UnsupportsCollection(Resource):
@@ -182,6 +186,7 @@ def test_retrieve(self):
 
             res = self.fetch("/api/v1/students/1/")
             self.assertEqual(res.code, httpstatus.NOT_FOUND)
+            self.assertNotIn("Content-Type", res.headers)
 
     def test_post_on_resource(self):
         with mock.patch("remoteappmanager.handlers.base_handler.BaseHandler"
@@ -348,6 +353,12 @@ def test_unprocessable(self):
                 body="{}"
             )
             self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(res.headers["Content-Type"], 'application/json')
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "BadRequest",
+                "message": "unprocessable",
+                "foo": "bar",
+            })
 
             res = self.fetch(
                 "/api/v1/unprocessables/0/",
@@ -355,6 +366,37 @@ def test_unprocessable(self):
                 body="{}"
             )
             self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(res.headers["Content-Type"], 'application/json')
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "BadRequest",
+                "message": "unprocessable",
+                "foo": "bar",
+            })
+
+            res = self.fetch(
+                "/api/v1/unprocessables/0/",
+                method="GET",
+            )
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(res.headers["Content-Type"], 'application/json')
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "BadRequest",
+                "message": "unprocessable",
+                "foo": "bar",
+            })
+
+            res = self.fetch(
+                "/api/v1/unprocessables/0/",
+                method="POST",
+                body="{}"
+            )
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(res.headers["Content-Type"], 'application/json')
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "BadRequest",
+                "message": "unprocessable",
+                "foo": "bar",
+            })
 
     def test_broken(self):
         collection_url = "/api/v1/brokens/"

From b70230bcd583fca4e2cb39e50cee70923c8dae0c Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 11:07:33 +0100
Subject: [PATCH 15/25] Added the items to the unprocessable

---
 tests/rest/test_rest.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/rest/test_rest.py b/tests/rest/test_rest.py
index ea5e3c5c4..5e45b5161 100644
--- a/tests/rest/test_rest.py
+++ b/tests/rest/test_rest.py
@@ -75,6 +75,10 @@ def update(self, identifier, representation):
     def retrieve(self, identifier):
         raise exceptions.BadRequest("unprocessable", foo="bar")
 
+    @gen.coroutine
+    def items(self):
+        raise exceptions.BadRequest("unprocessable", foo="bar")
+
 
 class UnsupportsCollection(Resource):
     @gen.coroutine
@@ -360,6 +364,18 @@ def test_unprocessable(self):
                 "foo": "bar",
             })
 
+            res = self.fetch(
+                "/api/v1/unprocessables/",
+                method="GET",
+            )
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(res.headers["Content-Type"], 'application/json')
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "BadRequest",
+                "message": "unprocessable",
+                "foo": "bar",
+            })
+
             res = self.fetch(
                 "/api/v1/unprocessables/0/",
                 method="PUT",

From 0ac6eae4ff55d10e71695a1d113cdc8ca5050dd0 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 11:15:06 +0100
Subject: [PATCH 16/25] Added tests for the container mapping id handling

---
 tests/restmodel/test_container.py | 42 +++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index ad3c05dd6..022d4438c 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -143,6 +143,48 @@ def test_create_fails(self):
             self.assertTrue(
                 self._app.container_manager.stop_and_remove_container.called)
 
+    def test_create_fails_for_missing_mapping_id(self):
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch(
+                "/api/v1/containers/",
+                method="POST",
+                body=escape.json_encode(dict(
+                    whatever="123"
+                )))
+
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(escape.json_decode(res.body),
+                             {"type": "BadRequest",
+                              "message": "missing mapping_id"})
+
+    def test_create_fails_for_invalid_mapping_id(self):
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch(
+                "/api/v1/containers/",
+                method="POST",
+                body=escape.json_encode(dict(
+                    mapping_id="whatever"
+                )))
+
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(escape.json_decode(res.body),
+                             {"type": "BadRequest",
+                              "message": "unrecognized mapping_id"})
+
     def test_retrieve(self):
         with patch("remoteappmanager"
                    ".handlers"

From 6e5ed8e2024d4d18f9c5da2488c38ab5219839af Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 15:31:57 +0100
Subject: [PATCH 17/25] Readapted tests for dummy mocks

---
 remoteappmanager/restresources/container.py |  24 +-
 tests/mocking/dummy.py                      |   2 +-
 tests/restmodel/test_container.py           | 312 +++++++++-----------
 3 files changed, 148 insertions(+), 190 deletions(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index dc7d289e0..4764b9544 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -68,7 +68,8 @@ def create(self, representation):
     @gen.coroutine
     def retrieve(self, identifier):
         """Return the representation of the running container."""
-        container = yield self._container_from_url_id(identifier)
+        container_manager = self.application.container_manager
+        container = yield container_manager.container_from_url_id(identifier)
 
         if container is None:
             self.log.warning("Could not find container for id {}".format(
@@ -83,8 +84,8 @@ def retrieve(self, identifier):
     @gen.coroutine
     def delete(self, identifier):
         """Stop the container."""
-        container = yield self._container_from_url_id(identifier)
         container_manager = self.application.container_manager
+        container = yield container_manager.container_from_url_id(identifier)
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
@@ -163,25 +164,6 @@ def _remove_container_on_error(self, container):
                         container.docker_id))
             raise e
 
-    @gen.coroutine
-    def _container_from_url_id(self, container_url_id):
-        """Retrieves and returns the container if valid and present.
-
-        If not present, returns None
-        """
-
-        container_manager = self.application.container_manager
-
-        container_dict = yield container_manager.docker_client.containers(
-            filters={'label': "{}={}".format(
-                SIMPHONY_NS+"url_id",
-                container_url_id)})
-
-        if not container_dict:
-            return None
-
-        return DockerContainer.from_docker_dict(container_dict[0])
-
     @gen.coroutine
     def _start_container(self, user_name, app, policy, mapping_id):
         """Start the container. This method is a helper method that
diff --git a/tests/mocking/dummy.py b/tests/mocking/dummy.py
index f5cb30f5e..b6725c427 100644
--- a/tests/mocking/dummy.py
+++ b/tests/mocking/dummy.py
@@ -102,7 +102,7 @@ def create_container_manager(params=None):
     manager.start_container = mock_coro_factory(Container())
     manager.stop_and_remove_container = mock_coro_factory()
     manager.containers_from_mapping_id = mock_coro_factory([Container()])
-    manager.container_from_url_is = mock_coro_factory(Container())
+    manager.container_from_url_id = mock_coro_factory(Container())
     manager.containers_from_filters = mock_coro_factory([Container()])
     manager.image = mock_coro_factory(Image())
     return manager
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index 022d4438c..eb9d4254e 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,131 +1,84 @@
+import os
 from unittest.mock import Mock, patch
 
 from remoteappmanager import rest
 from remoteappmanager.rest import registry
 from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.restresources import Container
+from remoteappmanager.docker.container import Container as DockerContainer
 from tests.mocking import dummy
+from tests.temp_mixin import TempMixin
 from tests.utils import (AsyncHTTPTestCase, mock_coro_factory,
-                         mock_coro_new_callable, containers_dict)
+                         mock_coro_new_callable)
 from tornado import web, escape
 
 
-class TestContainer(AsyncHTTPTestCase):
+class TestContainerApplication(TempMixin, AsyncHTTPTestCase):
     def setUp(self):
-        super().setUp()
-
-        def prepare_side_effect(*args, **kwargs):
-            user = Mock()
-            user.account = Mock()
-            args[0].current_user = user
+        self._old_proxy_api_token = os.environ.get("PROXY_API_TOKEN", None)
+        os.environ["PROXY_API_TOKEN"] = "dummy_token"
 
-        self.mock_prepare = mock_coro_new_callable(
-            side_effect=prepare_side_effect)
+        def cleanup():
+            if self._old_proxy_api_token is not None:
+                os.environ["PROXY_API_TOKEN"] = self._old_proxy_api_token
+            else:
+                del os.environ["PROXY_API_TOKEN"]
 
-    def get_app(self):
-        handlers = rest.api_handlers('/')
-        registry.registry.register(Container)
-        app = web.Application(handlers=handlers)
-        app.file_config = Mock()
-        app.file_config.network_timeout = 5
-        app.urlpath_for_object = Mock(return_value="/urlpath_for_object/")
-        app.command_line_config = Mock()
-        app.command_line_config.base_urlpath = "/"
-        app.reverse_proxy = dummy.create_reverse_proxy()
-        container = Mock()
-        container.urlpath = "containers/12345"
-        container.url_id = "12345"
-        app.container_manager = Mock()
-        app.container_manager.image = mock_coro_factory()
-        app.container_manager.start_container = mock_coro_factory(
-            return_value=container)
-        app.container_manager.stop_and_remove_container = mock_coro_factory()
-        app.container_manager.docker_client.containers = mock_coro_factory(
-            return_value=[]
-        )
+        self.addCleanup(cleanup)
 
-        app.db = Mock()
-        application_mock_1 = Mock()
-        application_mock_1.image = "hello1"
+        super().setUp()
 
-        application_mock_2 = Mock()
-        application_mock_2.image = "hello2"
-        app.db.get_apps_for_user = Mock(return_value=[
-            ("one", application_mock_1, Mock()),
-            ("two", application_mock_2, Mock()),
-        ])
+    def get_app(self):
+        app = dummy.create_application()
+        app.hub.verify_token.return_value = {
+            'pending': None,
+            'name': app.settings['user'],
+            'admin': False,
+            'server': app.settings['base_urlpath']}
         return app
 
     def test_items(self):
-        with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ):
-            res = self.fetch("/api/v1/containers/")
-
-            self.assertEqual(res.code, httpstatus.OK)
-            self.assertEqual(escape.json_decode(res.body),
-                             {"items": []})
-
-            # Add a mock image so that we check what happens if we do
-            # have something
-            self._app.container_manager.image = mock_coro_factory(
-                return_value=[Mock()]
-            )
-            mock_container = Mock()
-            mock_container.url_id = "hello"
-            self._app.container_manager.containers_from_mapping_id = \
-                mock_coro_factory([mock_container])
-
-            res = self.fetch("/api/v1/containers/")
-
-            self.assertEqual(res.code, httpstatus.OK)
-
-            # We have two "hello" but it's an artifact of the mocking.
-            # in a real application they are different, one for each
-            # application+policy running
-            self.assertEqual(escape.json_decode(res.body),
-                             {"items": ["hello", "hello"]})
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+        )
+
+        self.assertEqual(res.code, httpstatus.OK)
+
+        self.assertEqual(escape.json_decode(res.body),
+                         {"items": ["", ""]})
 
     def test_create(self):
         with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ), \
-             patch("remoteappmanager"
                    ".restresources"
                    ".container"
                    ".wait_for_http_server_2xx",
                    new_callable=mock_coro_new_callable()):
 
+            manager = self._app.container_manager
+            manager.start_container = mock_coro_factory(DockerContainer(
+                url_id="3456"
+            ))
             res = self.fetch(
-                "/api/v1/containers/",
+                "/user/username/api/v1/containers/",
                 method="POST",
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
                 body=escape.json_encode(dict(
-                    mapping_id="one"
+                    mapping_id="12345"
                 )))
 
             self.assertEqual(res.code, httpstatus.CREATED)
 
             # The port is random due to testing env. Check if it's absolute
             self.assertIn("http://", res.headers["Location"])
-            self.assertIn("/api/v1/containers/12345/", res.headers["Location"])
+            self.assertIn("/api/v1/containers/3456/", res.headers["Location"])
 
     def test_create_fails(self):
         with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ), \
-             patch("remoteappmanager"
                    ".restresources"
                    ".container"
                    ".wait_for_http_server_2xx",
@@ -133,10 +86,13 @@ def test_create_fails(self):
                        side_effect=TimeoutError())):
 
             res = self.fetch(
-                "/api/v1/containers/",
+                "/user/username/api/v1/containers/",
                 method="POST",
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
                 body=escape.json_encode(dict(
-                    mapping_id="one"
+                    mapping_id="12345"
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -144,87 +100,107 @@ def test_create_fails(self):
                 self._app.container_manager.stop_and_remove_container.called)
 
     def test_create_fails_for_missing_mapping_id(self):
-        with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ):
-
-            res = self.fetch(
-                "/api/v1/containers/",
-                method="POST",
-                body=escape.json_encode(dict(
-                    whatever="123"
-                )))
-
-            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
-            self.assertEqual(escape.json_decode(res.body),
-                             {"type": "BadRequest",
-                              "message": "missing mapping_id"})
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            method="POST",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+            body=escape.json_encode(dict(
+                whatever="123"
+            )))
+
+        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+        self.assertEqual(escape.json_decode(res.body),
+                         {"type": "BadRequest",
+                          "message": "missing mapping_id"})
 
     def test_create_fails_for_invalid_mapping_id(self):
-        with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ):
-
-            res = self.fetch(
-                "/api/v1/containers/",
-                method="POST",
-                body=escape.json_encode(dict(
-                    mapping_id="whatever"
-                )))
-
-            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
-            self.assertEqual(escape.json_decode(res.body),
-                             {"type": "BadRequest",
-                              "message": "unrecognized mapping_id"})
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            method="POST",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+            body=escape.json_encode(dict(
+                mapping_id="whatever"
+            )))
+
+        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+        self.assertEqual(escape.json_decode(res.body),
+                         {"type": "BadRequest",
+                          "message": "unrecognized mapping_id"})
 
     def test_retrieve(self):
-        with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ):
-
-            res = self.fetch("/api/v1/containers/notfound/")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
-
-            self._app.container_manager.docker_client.containers = \
-                mock_coro_factory(return_value=[containers_dict()])
-
-            # The url is not important. The replacement of the containers
-            # method up there guarantees that the method will return
-            # something, regardless of the filter used.
-            res = self.fetch("/api/v1/containers/found/")
-            self.assertEqual(res.code, httpstatus.OK)
-
-            content = escape.json_decode(res.body)
-            self.assertEqual(content["image_name"],
-                             "simphony/app:simphony-framework-mayavi")
-            self.assertEqual(content["name"], "/cocky_pasteur")
+        res = self.fetch("/user/username/api/v1/containers/found/",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.OK)
+
+        content = escape.json_decode(res.body)
+        self.assertEqual(content["image_name"], "")
+        self.assertEqual(content["name"], "")
+
+        self._app.container_manager.container_from_url_id = \
+            mock_coro_factory(return_value=None)
+        res = self.fetch("/user/username/api/v1/containers/notfound/",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NOT_FOUND)
 
     def test_delete(self):
+        res = self.fetch("/user/username/api/v1/containers/found/",
+                         method="DELETE",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NO_CONTENT)
+
+        self._app.container_manager.container_from_url_id = \
+            mock_coro_factory(return_value=None)
+        res = self.fetch("/user/username/api/v1/containers/notfound/",
+                         method="DELETE",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NOT_FOUND)
+
+    def test_post_start(self):
         with patch("remoteappmanager"
-                   ".handlers"
-                   ".base_handler"
-                   ".BaseHandler"
-                   ".prepare",
-                   new_callable=self.mock_prepare
-                   ):
-
-            res = self.fetch("/api/v1/containers/notfound/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
-
-            self._app.container_manager.docker_client.containers = \
-                mock_coro_factory(return_value=[containers_dict()])
-
-            res = self.fetch("/api/v1/containers/found/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NO_CONTENT)
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_factory):
+
+            self.assertFalse(self._app.reverse_proxy.register.called)
+            self.fetch("/user/username/api/v1/containers/",
+                       method="POST",
+                       headers={
+                                "Cookie": "jupyter-hub-token-username=foo"
+                       },
+                       body=escape.json_encode({"mapping_id": "12345"}))
+
+            self.assertTrue(self._app.reverse_proxy.register.called)
+
+    def test_post_failed_auth(self):
+        self._app.hub.verify_token.return_value = {}
+
+        res = self.fetch("/user/username/api/v1/containers/",
+                         method="POST",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         },
+                         body=escape.json_encode({"mapping_id": "12345"}))
+
+        self.assertGreaterEqual(res.code, 400)
+
+    def test_stop(self):
+        self.fetch("/user/username/api/v1/containers/12345/",
+                   method="DELETE",
+                   headers={
+                      "Cookie": "jupyter-hub-token-username=foo"
+                   })
+
+        self.assertTrue(self._app.reverse_proxy.unregister.called)

From 7bb495dd824b027a127b84bbe434f984fc30fec4 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 15:36:37 +0100
Subject: [PATCH 18/25] Revert "Readapted tests for dummy mocks"

This reverts commit 6e5ed8e2024d4d18f9c5da2488c38ab5219839af.
---
 remoteappmanager/restresources/container.py |  24 +-
 tests/mocking/dummy.py                      |   2 +-
 tests/restmodel/test_container.py           | 312 +++++++++++---------
 3 files changed, 190 insertions(+), 148 deletions(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 4764b9544..dc7d289e0 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -68,8 +68,7 @@ def create(self, representation):
     @gen.coroutine
     def retrieve(self, identifier):
         """Return the representation of the running container."""
-        container_manager = self.application.container_manager
-        container = yield container_manager.container_from_url_id(identifier)
+        container = yield self._container_from_url_id(identifier)
 
         if container is None:
             self.log.warning("Could not find container for id {}".format(
@@ -84,8 +83,8 @@ def retrieve(self, identifier):
     @gen.coroutine
     def delete(self, identifier):
         """Stop the container."""
+        container = yield self._container_from_url_id(identifier)
         container_manager = self.application.container_manager
-        container = yield container_manager.container_from_url_id(identifier)
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
@@ -164,6 +163,25 @@ def _remove_container_on_error(self, container):
                         container.docker_id))
             raise e
 
+    @gen.coroutine
+    def _container_from_url_id(self, container_url_id):
+        """Retrieves and returns the container if valid and present.
+
+        If not present, returns None
+        """
+
+        container_manager = self.application.container_manager
+
+        container_dict = yield container_manager.docker_client.containers(
+            filters={'label': "{}={}".format(
+                SIMPHONY_NS+"url_id",
+                container_url_id)})
+
+        if not container_dict:
+            return None
+
+        return DockerContainer.from_docker_dict(container_dict[0])
+
     @gen.coroutine
     def _start_container(self, user_name, app, policy, mapping_id):
         """Start the container. This method is a helper method that
diff --git a/tests/mocking/dummy.py b/tests/mocking/dummy.py
index b6725c427..f5cb30f5e 100644
--- a/tests/mocking/dummy.py
+++ b/tests/mocking/dummy.py
@@ -102,7 +102,7 @@ def create_container_manager(params=None):
     manager.start_container = mock_coro_factory(Container())
     manager.stop_and_remove_container = mock_coro_factory()
     manager.containers_from_mapping_id = mock_coro_factory([Container()])
-    manager.container_from_url_id = mock_coro_factory(Container())
+    manager.container_from_url_is = mock_coro_factory(Container())
     manager.containers_from_filters = mock_coro_factory([Container()])
     manager.image = mock_coro_factory(Image())
     return manager
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index eb9d4254e..022d4438c 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,84 +1,131 @@
-import os
 from unittest.mock import Mock, patch
 
 from remoteappmanager import rest
 from remoteappmanager.rest import registry
 from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.restresources import Container
-from remoteappmanager.docker.container import Container as DockerContainer
 from tests.mocking import dummy
-from tests.temp_mixin import TempMixin
 from tests.utils import (AsyncHTTPTestCase, mock_coro_factory,
-                         mock_coro_new_callable)
+                         mock_coro_new_callable, containers_dict)
 from tornado import web, escape
 
 
-class TestContainerApplication(TempMixin, AsyncHTTPTestCase):
+class TestContainer(AsyncHTTPTestCase):
     def setUp(self):
-        self._old_proxy_api_token = os.environ.get("PROXY_API_TOKEN", None)
-        os.environ["PROXY_API_TOKEN"] = "dummy_token"
-
-        def cleanup():
-            if self._old_proxy_api_token is not None:
-                os.environ["PROXY_API_TOKEN"] = self._old_proxy_api_token
-            else:
-                del os.environ["PROXY_API_TOKEN"]
+        super().setUp()
 
-        self.addCleanup(cleanup)
+        def prepare_side_effect(*args, **kwargs):
+            user = Mock()
+            user.account = Mock()
+            args[0].current_user = user
 
-        super().setUp()
+        self.mock_prepare = mock_coro_new_callable(
+            side_effect=prepare_side_effect)
 
     def get_app(self):
-        app = dummy.create_application()
-        app.hub.verify_token.return_value = {
-            'pending': None,
-            'name': app.settings['user'],
-            'admin': False,
-            'server': app.settings['base_urlpath']}
-        return app
-
-    def test_items(self):
-        res = self.fetch(
-            "/user/username/api/v1/containers/",
-            headers={
-                "Cookie": "jupyter-hub-token-username=foo"
-            },
+        handlers = rest.api_handlers('/')
+        registry.registry.register(Container)
+        app = web.Application(handlers=handlers)
+        app.file_config = Mock()
+        app.file_config.network_timeout = 5
+        app.urlpath_for_object = Mock(return_value="/urlpath_for_object/")
+        app.command_line_config = Mock()
+        app.command_line_config.base_urlpath = "/"
+        app.reverse_proxy = dummy.create_reverse_proxy()
+        container = Mock()
+        container.urlpath = "containers/12345"
+        container.url_id = "12345"
+        app.container_manager = Mock()
+        app.container_manager.image = mock_coro_factory()
+        app.container_manager.start_container = mock_coro_factory(
+            return_value=container)
+        app.container_manager.stop_and_remove_container = mock_coro_factory()
+        app.container_manager.docker_client.containers = mock_coro_factory(
+            return_value=[]
         )
 
-        self.assertEqual(res.code, httpstatus.OK)
+        app.db = Mock()
+        application_mock_1 = Mock()
+        application_mock_1.image = "hello1"
+
+        application_mock_2 = Mock()
+        application_mock_2.image = "hello2"
+        app.db.get_apps_for_user = Mock(return_value=[
+            ("one", application_mock_1, Mock()),
+            ("two", application_mock_2, Mock()),
+        ])
+        return app
 
-        self.assertEqual(escape.json_decode(res.body),
-                         {"items": ["", ""]})
+    def test_items(self):
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+            res = self.fetch("/api/v1/containers/")
+
+            self.assertEqual(res.code, httpstatus.OK)
+            self.assertEqual(escape.json_decode(res.body),
+                             {"items": []})
+
+            # Add a mock image so that we check what happens if we do
+            # have something
+            self._app.container_manager.image = mock_coro_factory(
+                return_value=[Mock()]
+            )
+            mock_container = Mock()
+            mock_container.url_id = "hello"
+            self._app.container_manager.containers_from_mapping_id = \
+                mock_coro_factory([mock_container])
+
+            res = self.fetch("/api/v1/containers/")
+
+            self.assertEqual(res.code, httpstatus.OK)
+
+            # We have two "hello" but it's an artifact of the mocking.
+            # in a real application they are different, one for each
+            # application+policy running
+            self.assertEqual(escape.json_decode(res.body),
+                             {"items": ["hello", "hello"]})
 
     def test_create(self):
         with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ), \
+             patch("remoteappmanager"
                    ".restresources"
                    ".container"
                    ".wait_for_http_server_2xx",
                    new_callable=mock_coro_new_callable()):
 
-            manager = self._app.container_manager
-            manager.start_container = mock_coro_factory(DockerContainer(
-                url_id="3456"
-            ))
             res = self.fetch(
-                "/user/username/api/v1/containers/",
+                "/api/v1/containers/",
                 method="POST",
-                headers={
-                    "Cookie": "jupyter-hub-token-username=foo"
-                },
                 body=escape.json_encode(dict(
-                    mapping_id="12345"
+                    mapping_id="one"
                 )))
 
             self.assertEqual(res.code, httpstatus.CREATED)
 
             # The port is random due to testing env. Check if it's absolute
             self.assertIn("http://", res.headers["Location"])
-            self.assertIn("/api/v1/containers/3456/", res.headers["Location"])
+            self.assertIn("/api/v1/containers/12345/", res.headers["Location"])
 
     def test_create_fails(self):
         with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ), \
+             patch("remoteappmanager"
                    ".restresources"
                    ".container"
                    ".wait_for_http_server_2xx",
@@ -86,13 +133,10 @@ def test_create_fails(self):
                        side_effect=TimeoutError())):
 
             res = self.fetch(
-                "/user/username/api/v1/containers/",
+                "/api/v1/containers/",
                 method="POST",
-                headers={
-                    "Cookie": "jupyter-hub-token-username=foo"
-                },
                 body=escape.json_encode(dict(
-                    mapping_id="12345"
+                    mapping_id="one"
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -100,107 +144,87 @@ def test_create_fails(self):
                 self._app.container_manager.stop_and_remove_container.called)
 
     def test_create_fails_for_missing_mapping_id(self):
-        res = self.fetch(
-            "/user/username/api/v1/containers/",
-            method="POST",
-            headers={
-                "Cookie": "jupyter-hub-token-username=foo"
-            },
-            body=escape.json_encode(dict(
-                whatever="123"
-            )))
-
-        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
-        self.assertEqual(escape.json_decode(res.body),
-                         {"type": "BadRequest",
-                          "message": "missing mapping_id"})
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch(
+                "/api/v1/containers/",
+                method="POST",
+                body=escape.json_encode(dict(
+                    whatever="123"
+                )))
+
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(escape.json_decode(res.body),
+                             {"type": "BadRequest",
+                              "message": "missing mapping_id"})
 
     def test_create_fails_for_invalid_mapping_id(self):
-        res = self.fetch(
-            "/user/username/api/v1/containers/",
-            method="POST",
-            headers={
-                "Cookie": "jupyter-hub-token-username=foo"
-            },
-            body=escape.json_encode(dict(
-                mapping_id="whatever"
-            )))
-
-        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
-        self.assertEqual(escape.json_decode(res.body),
-                         {"type": "BadRequest",
-                          "message": "unrecognized mapping_id"})
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch(
+                "/api/v1/containers/",
+                method="POST",
+                body=escape.json_encode(dict(
+                    mapping_id="whatever"
+                )))
+
+            self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+            self.assertEqual(escape.json_decode(res.body),
+                             {"type": "BadRequest",
+                              "message": "unrecognized mapping_id"})
 
     def test_retrieve(self):
-        res = self.fetch("/user/username/api/v1/containers/found/",
-                         headers={
-                             "Cookie": "jupyter-hub-token-username=foo"
-                         })
-        self.assertEqual(res.code, httpstatus.OK)
-
-        content = escape.json_decode(res.body)
-        self.assertEqual(content["image_name"], "")
-        self.assertEqual(content["name"], "")
-
-        self._app.container_manager.container_from_url_id = \
-            mock_coro_factory(return_value=None)
-        res = self.fetch("/user/username/api/v1/containers/notfound/",
-                         headers={
-                             "Cookie": "jupyter-hub-token-username=foo"
-                         })
-        self.assertEqual(res.code, httpstatus.NOT_FOUND)
+        with patch("remoteappmanager"
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch("/api/v1/containers/notfound/")
+            self.assertEqual(res.code, httpstatus.NOT_FOUND)
+
+            self._app.container_manager.docker_client.containers = \
+                mock_coro_factory(return_value=[containers_dict()])
+
+            # The url is not important. The replacement of the containers
+            # method up there guarantees that the method will return
+            # something, regardless of the filter used.
+            res = self.fetch("/api/v1/containers/found/")
+            self.assertEqual(res.code, httpstatus.OK)
+
+            content = escape.json_decode(res.body)
+            self.assertEqual(content["image_name"],
+                             "simphony/app:simphony-framework-mayavi")
+            self.assertEqual(content["name"], "/cocky_pasteur")
 
     def test_delete(self):
-        res = self.fetch("/user/username/api/v1/containers/found/",
-                         method="DELETE",
-                         headers={
-                             "Cookie": "jupyter-hub-token-username=foo"
-                         })
-        self.assertEqual(res.code, httpstatus.NO_CONTENT)
-
-        self._app.container_manager.container_from_url_id = \
-            mock_coro_factory(return_value=None)
-        res = self.fetch("/user/username/api/v1/containers/notfound/",
-                         method="DELETE",
-                         headers={
-                             "Cookie": "jupyter-hub-token-username=foo"
-                         })
-        self.assertEqual(res.code, httpstatus.NOT_FOUND)
-
-    def test_post_start(self):
         with patch("remoteappmanager"
-                   ".restresources"
-                   ".container"
-                   ".wait_for_http_server_2xx",
-                   new_callable=mock_coro_factory):
-
-            self.assertFalse(self._app.reverse_proxy.register.called)
-            self.fetch("/user/username/api/v1/containers/",
-                       method="POST",
-                       headers={
-                                "Cookie": "jupyter-hub-token-username=foo"
-                       },
-                       body=escape.json_encode({"mapping_id": "12345"}))
-
-            self.assertTrue(self._app.reverse_proxy.register.called)
-
-    def test_post_failed_auth(self):
-        self._app.hub.verify_token.return_value = {}
-
-        res = self.fetch("/user/username/api/v1/containers/",
-                         method="POST",
-                         headers={
-                             "Cookie": "jupyter-hub-token-username=foo"
-                         },
-                         body=escape.json_encode({"mapping_id": "12345"}))
-
-        self.assertGreaterEqual(res.code, 400)
-
-    def test_stop(self):
-        self.fetch("/user/username/api/v1/containers/12345/",
-                   method="DELETE",
-                   headers={
-                      "Cookie": "jupyter-hub-token-username=foo"
-                   })
-
-        self.assertTrue(self._app.reverse_proxy.unregister.called)
+                   ".handlers"
+                   ".base_handler"
+                   ".BaseHandler"
+                   ".prepare",
+                   new_callable=self.mock_prepare
+                   ):
+
+            res = self.fetch("/api/v1/containers/notfound/", method="DELETE")
+            self.assertEqual(res.code, httpstatus.NOT_FOUND)
+
+            self._app.container_manager.docker_client.containers = \
+                mock_coro_factory(return_value=[containers_dict()])
+
+            res = self.fetch("/api/v1/containers/found/", method="DELETE")
+            self.assertEqual(res.code, httpstatus.NO_CONTENT)

From 84df4e9a8c13be5706efe72ce3c34fbf8a3d7942 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 15:31:57 +0100
Subject: [PATCH 19/25] Readapted tests for dummy mocks

---
 remoteappmanager/restresources/container.py | 24 +++------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index dc7d289e0..4764b9544 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -68,7 +68,8 @@ def create(self, representation):
     @gen.coroutine
     def retrieve(self, identifier):
         """Return the representation of the running container."""
-        container = yield self._container_from_url_id(identifier)
+        container_manager = self.application.container_manager
+        container = yield container_manager.container_from_url_id(identifier)
 
         if container is None:
             self.log.warning("Could not find container for id {}".format(
@@ -83,8 +84,8 @@ def retrieve(self, identifier):
     @gen.coroutine
     def delete(self, identifier):
         """Stop the container."""
-        container = yield self._container_from_url_id(identifier)
         container_manager = self.application.container_manager
+        container = yield container_manager.container_from_url_id(identifier)
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
@@ -163,25 +164,6 @@ def _remove_container_on_error(self, container):
                         container.docker_id))
             raise e
 
-    @gen.coroutine
-    def _container_from_url_id(self, container_url_id):
-        """Retrieves and returns the container if valid and present.
-
-        If not present, returns None
-        """
-
-        container_manager = self.application.container_manager
-
-        container_dict = yield container_manager.docker_client.containers(
-            filters={'label': "{}={}".format(
-                SIMPHONY_NS+"url_id",
-                container_url_id)})
-
-        if not container_dict:
-            return None
-
-        return DockerContainer.from_docker_dict(container_dict[0])
-
     @gen.coroutine
     def _start_container(self, user_name, app, policy, mapping_id):
         """Start the container. This method is a helper method that

From 246113d74dc4112065a356b6cdb03f414dbed84d Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 16:01:00 +0100
Subject: [PATCH 20/25] Adapted tests

---
 remoteappmanager/restresources/container.py |   2 -
 tests/mocking/dummy.py                      |  16 +-
 tests/restmodel/test_container.py           | 250 +++++++++++++-------
 3 files changed, 180 insertions(+), 88 deletions(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 4764b9544..191c95234 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -4,10 +4,8 @@
 
 from tornado import gen
 
-from remoteappmanager.docker.docker_labels import SIMPHONY_NS
 from remoteappmanager.rest import exceptions
 from remoteappmanager.rest.resource import Resource
-from remoteappmanager.docker.container import Container as DockerContainer
 from remoteappmanager.utils import url_path_join
 from remoteappmanager.netutils import wait_for_http_server_2xx
 
diff --git a/tests/mocking/dummy.py b/tests/mocking/dummy.py
index 0a8030629..b6725c427 100644
--- a/tests/mocking/dummy.py
+++ b/tests/mocking/dummy.py
@@ -4,9 +4,10 @@
 from remoteappmanager.application import Application
 from remoteappmanager.db import interfaces
 from remoteappmanager.docker.container_manager import ContainerManager
+from remoteappmanager.docker.container import Container
+from remoteappmanager.docker.image import Image
 
 from tests.utils import mock_coro_factory, basic_command_line_config
-from tests.mocking.virtual.docker_client import create_docker_client
 
 
 class DummyDBApplication(interfaces.ABCApplication):
@@ -23,11 +24,11 @@ def get_user_by_name(self, user_name):
         return user_name
 
     def get_apps_for_user(self, account):
-        return (('mapping_id',
-                 DummyDBApplication(image='image_id1'),
+        return (('12345',
+                 DummyDBApplication(image='image1'),
                  DummyDBApplicationPolicy()),
                 ('id678',
-                 DummyDBApplication(image='image_id1'),
+                 DummyDBApplication(image='image2'),
                  DummyDBApplicationPolicy()))
 
 
@@ -98,7 +99,12 @@ def create_container_manager(params=None):
         params = {'docker_config': {}}
 
     manager = ContainerManager(**params)
-    manager.docker_client._sync_client = create_docker_client()
+    manager.start_container = mock_coro_factory(Container())
+    manager.stop_and_remove_container = mock_coro_factory()
+    manager.containers_from_mapping_id = mock_coro_factory([Container()])
+    manager.container_from_url_id = mock_coro_factory(Container())
+    manager.containers_from_filters = mock_coro_factory([Container()])
+    manager.image = mock_coro_factory(Image())
     return manager
 
 
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index e125b136e..c1c77b29a 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,115 +1,203 @@
-from unittest.mock import Mock, patch
-
-from tornado import escape
-
-from remoteappmanager.rest import httpstatus
+import os
+from unittest.mock import patch
 
+from remoteappmanager.rest.http import httpstatus
+from remoteappmanager.docker.container import Container as DockerContainer
+from tests.mocking import dummy
+from tests.temp_mixin import TempMixin
 from tests.utils import (AsyncHTTPTestCase, mock_coro_factory,
                          mock_coro_new_callable)
-from tests.mocking import dummy
-from tests.mocking.virtual.docker_client import create_docker_client
+from tornado import escape
 
 
-class TestContainer(AsyncHTTPTestCase):
+class TestContainerApplication(TempMixin, AsyncHTTPTestCase):
     def setUp(self):
-        super().setUp()
+        self._old_proxy_api_token = os.environ.get("PROXY_API_TOKEN", None)
+        os.environ["PROXY_API_TOKEN"] = "dummy_token"
 
-        def prepare_side_effect(*args, **kwargs):
-            user = Mock()
-            user.name = 'user_name'
-            args[0].current_user = user
+        def cleanup():
+            if self._old_proxy_api_token is not None:
+                os.environ["PROXY_API_TOKEN"] = self._old_proxy_api_token
+            else:
+                del os.environ["PROXY_API_TOKEN"]
 
-        self.mock_prepare = mock_coro_new_callable(
-            side_effect=prepare_side_effect)
+        self.addCleanup(cleanup)
+
+        super().setUp()
 
     def get_app(self):
-        command_line_config = dummy.basic_command_line_config()
-        command_line_config.base_urlpath = '/'
-        return dummy.create_application(command_line_config)
+        app = dummy.create_application()
+        app.hub.verify_token.return_value = {
+            'pending': None,
+            'name': app.settings['user'],
+            'admin': False,
+            'server': app.settings['base_urlpath']}
+        return app
 
     def test_items(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",   # noqa
-                   new_callable=self.mock_prepare):
-            res = self.fetch("/api/v1/containers/")
-
-            self.assertEqual(res.code, httpstatus.OK)
-            self.assertEqual(escape.json_decode(res.body),
-                             {"items": ['url_id']})
-
-            # We have another container running
-            self._app.container_manager.docker_client._sync_client = (
-                create_docker_client(
-                    container_ids=('container_id1',),
-                    container_labels=(
-                        {'eu.simphony-project.docker.user': 'user_name',
-                         'eu.simphony-project.docker.mapping_id': 'mapping_id',
-                         'eu.simphony-project.docker.url_id': 'url_id1234'},)))
-
-            res = self.fetch("/api/v1/containers/")
-            self.assertEqual(res.code, httpstatus.OK)
-            self.assertEqual(escape.json_decode(res.body),
-                             {"items": ["url_id1234"]})
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+        )
 
-    def test_create(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",  # noqa
-                   new_callable=self.mock_prepare), \
-                patch("remoteappmanager.restresources.container.wait_for_http_server_2xx",  # noqa
-                      new_callable=mock_coro_factory), \
-                patch("remoteappmanager.docker.container_manager._generate_container_url_id",  # noqa
-                      return_value="12345678"):
+        self.assertEqual(res.code, httpstatus.OK)
 
+        self.assertEqual(escape.json_decode(res.body),
+                         {"items": ["", ""]})
+
+    def test_create(self):
+        with patch("remoteappmanager"
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_new_callable()):
+
+            manager = self._app.container_manager
+            manager.start_container = mock_coro_factory(DockerContainer(
+                url_id="3456"
+            ))
             res = self.fetch(
-                "/api/v1/containers/",
+                "/user/username/api/v1/containers/",
                 method="POST",
-                body=escape.json_encode({'mapping_id': 'mapping_id'}))
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
+                body=escape.json_encode(dict(
+                    mapping_id="12345"
+                )))
 
             self.assertEqual(res.code, httpstatus.CREATED)
 
             # The port is random due to testing env. Check if it's absolute
             self.assertIn("http://", res.headers["Location"])
-            self.assertIn("/api/v1/containers/12345678",
-                          res.headers["Location"])
+            self.assertIn("/api/v1/containers/3456/", res.headers["Location"])
 
     def test_create_fails(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",   # noqa
-                   new_callable=self.mock_prepare), \
-                patch("remoteappmanager.restresources.container.wait_for_http_server_2xx",   # noqa
-                      new_callable=mock_coro_new_callable(
-                          side_effect=TimeoutError())):
+        with patch("remoteappmanager"
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_new_callable(
+                       side_effect=TimeoutError())):
 
             res = self.fetch(
-                "/api/v1/containers/",
+                "/user/username/api/v1/containers/",
                 method="POST",
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
                 body=escape.json_encode(dict(
-                    mapping_id="mapping_id"
+                    mapping_id="12345"
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
-            client = self._app.container_manager.docker_client._sync_client
-            self.assertTrue(client.stop.called)
-            self.assertTrue(client.remove_container.called)
+            self.assertTrue(
+                self._app.container_manager.stop_and_remove_container.called)
+
+    def test_create_fails_for_missing_mapping_id(self):
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            method="POST",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+            body=escape.json_encode(dict(
+                whatever="123"
+            )))
+
+        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+        self.assertEqual(escape.json_decode(res.body),
+                         {"type": "BadRequest",
+                          "message": "missing mapping_id"})
+
+    def test_create_fails_for_invalid_mapping_id(self):
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            method="POST",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+            body=escape.json_encode(dict(
+                mapping_id="whatever"
+            )))
+
+        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+        self.assertEqual(escape.json_decode(res.body),
+                         {"type": "BadRequest",
+                          "message": "unrecognized mapping_id"})
 
     def test_retrieve(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",  # noqa
-                   new_callable=self.mock_prepare):
-
-            res = self.fetch("/api/v1/containers/notfound/")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
-
-            res = self.fetch("/api/v1/containers/url_id/")
-            self.assertEqual(res.code, httpstatus.OK)
-
-            content = escape.json_decode(res.body)
-            self.assertEqual(content["image_name"], "image_name1")
-            self.assertEqual(content["name"],
-                             "/remoteexec-username-mapping_5Fid")
+        res = self.fetch("/user/username/api/v1/containers/found/",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.OK)
+
+        content = escape.json_decode(res.body)
+        self.assertEqual(content["image_name"], "")
+        self.assertEqual(content["name"], "")
+
+        self._app.container_manager.container_from_url_id = \
+            mock_coro_factory(return_value=None)
+        res = self.fetch("/user/username/api/v1/containers/notfound/",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NOT_FOUND)
 
     def test_delete(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",  # noqa
-                   new_callable=self.mock_prepare):
-
-            res = self.fetch("/api/v1/containers/notfound/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
-
-            res = self.fetch("/api/v1/containers/url_id/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NO_CONTENT)
+        res = self.fetch("/user/username/api/v1/containers/found/",
+                         method="DELETE",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NO_CONTENT)
+
+        self._app.container_manager.container_from_url_id = \
+            mock_coro_factory(return_value=None)
+        res = self.fetch("/user/username/api/v1/containers/notfound/",
+                         method="DELETE",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         })
+        self.assertEqual(res.code, httpstatus.NOT_FOUND)
+
+    def test_post_start(self):
+        with patch("remoteappmanager"
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_factory):
+
+            self.assertFalse(self._app.reverse_proxy.register.called)
+            self.fetch("/user/username/api/v1/containers/",
+                       method="POST",
+                       headers={
+                                "Cookie": "jupyter-hub-token-username=foo"
+                       },
+                       body=escape.json_encode({"mapping_id": "12345"}))
+
+            self.assertTrue(self._app.reverse_proxy.register.called)
+
+    def test_post_failed_auth(self):
+        self._app.hub.verify_token.return_value = {}
+
+        res = self.fetch("/user/username/api/v1/containers/",
+                         method="POST",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         },
+                         body=escape.json_encode({"mapping_id": "12345"}))
+
+        self.assertGreaterEqual(res.code, 400)
+
+    def test_stop(self):
+        self.fetch("/user/username/api/v1/containers/12345/",
+                   method="DELETE",
+                   headers={
+                      "Cookie": "jupyter-hub-token-username=foo"
+                   })
+
+        self.assertTrue(self._app.reverse_proxy.unregister.called)

From e9195989c2b1b0a5a45866240ee3d799f6df93bf Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 16:02:09 +0100
Subject: [PATCH 21/25] Neutralized apidoc change

---
 doc/source/api/remoteappmanager.db.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/source/api/remoteappmanager.db.rst b/doc/source/api/remoteappmanager.db.rst
index 411898e03..e33719b6c 100644
--- a/doc/source/api/remoteappmanager.db.rst
+++ b/doc/source/api/remoteappmanager.db.rst
@@ -9,6 +9,7 @@ remoteappmanager.db.csv_db module
 
 .. automodule:: remoteappmanager.db.csv_db
     :members:
+    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -17,6 +18,7 @@ remoteappmanager.db.interfaces module
 
 .. automodule:: remoteappmanager.db.interfaces
     :members:
+    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -25,6 +27,7 @@ remoteappmanager.db.orm module
 
 .. automodule:: remoteappmanager.db.orm
     :members:
+    :special-members: __init__
     :undoc-members:
     :show-inheritance:
 
@@ -34,5 +37,6 @@ Module contents
 
 .. automodule:: remoteappmanager.db
     :members:
+    :special-members: __init__
     :undoc-members:
     :show-inheritance:

From da7287f7972c3c77b8dca874424dd2ce2ff394a9 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 16:24:01 +0100
Subject: [PATCH 22/25] Restored dummy

---
 tests/mocking/dummy.py | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/tests/mocking/dummy.py b/tests/mocking/dummy.py
index b6725c427..0a8030629 100644
--- a/tests/mocking/dummy.py
+++ b/tests/mocking/dummy.py
@@ -4,10 +4,9 @@
 from remoteappmanager.application import Application
 from remoteappmanager.db import interfaces
 from remoteappmanager.docker.container_manager import ContainerManager
-from remoteappmanager.docker.container import Container
-from remoteappmanager.docker.image import Image
 
 from tests.utils import mock_coro_factory, basic_command_line_config
+from tests.mocking.virtual.docker_client import create_docker_client
 
 
 class DummyDBApplication(interfaces.ABCApplication):
@@ -24,11 +23,11 @@ def get_user_by_name(self, user_name):
         return user_name
 
     def get_apps_for_user(self, account):
-        return (('12345',
-                 DummyDBApplication(image='image1'),
+        return (('mapping_id',
+                 DummyDBApplication(image='image_id1'),
                  DummyDBApplicationPolicy()),
                 ('id678',
-                 DummyDBApplication(image='image2'),
+                 DummyDBApplication(image='image_id1'),
                  DummyDBApplicationPolicy()))
 
 
@@ -99,12 +98,7 @@ def create_container_manager(params=None):
         params = {'docker_config': {}}
 
     manager = ContainerManager(**params)
-    manager.start_container = mock_coro_factory(Container())
-    manager.stop_and_remove_container = mock_coro_factory()
-    manager.containers_from_mapping_id = mock_coro_factory([Container()])
-    manager.container_from_url_id = mock_coro_factory(Container())
-    manager.containers_from_filters = mock_coro_factory([Container()])
-    manager.image = mock_coro_factory(Image())
+    manager.docker_client._sync_client = create_docker_client()
     return manager
 
 

From 446fe7e9a052bf0cc5a7efde00a94c2dcd631428 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 16:26:01 +0100
Subject: [PATCH 23/25] Changed test name

---
 tests/restmodel/test_container.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index c1c77b29a..5d12c3b99 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -10,7 +10,7 @@
 from tornado import escape
 
 
-class TestContainerApplication(TempMixin, AsyncHTTPTestCase):
+class TestContainer(TempMixin, AsyncHTTPTestCase):
     def setUp(self):
         self._old_proxy_api_token = os.environ.get("PROXY_API_TOKEN", None)
         os.environ["PROXY_API_TOKEN"] = "dummy_token"

From 64ea0b11ebc604575c6acd63b94928810bc030b5 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 17:14:42 +0100
Subject: [PATCH 24/25] Regreened tests

---
 tests/restmodel/test_container.py | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index 5d12c3b99..aa0d26204 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,6 +1,7 @@
 import os
 from unittest.mock import patch
 
+from remoteappmanager.docker.image import Image
 from remoteappmanager.rest.http import httpstatus
 from remoteappmanager.docker.container import Container as DockerContainer
 from tests.mocking import dummy
@@ -35,6 +36,11 @@ def get_app(self):
         return app
 
     def test_items(self):
+        manager = self._app.container_manager
+        manager.image = mock_coro_factory(Image())
+        manager.containers_from_mapping_id = mock_coro_factory(
+            [DockerContainer()])
+
         res = self.fetch(
             "/user/username/api/v1/containers/",
             headers={
@@ -65,7 +71,7 @@ def test_create(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="12345"
+                    mapping_id="mapping_id"
                 )))
 
             self.assertEqual(res.code, httpstatus.CREATED)
@@ -82,6 +88,8 @@ def test_create_fails(self):
                    new_callable=mock_coro_new_callable(
                        side_effect=TimeoutError())):
 
+            self._app.container_manager.stop_and_remove_container = \
+                mock_coro_factory()
             res = self.fetch(
                 "/user/username/api/v1/containers/",
                 method="POST",
@@ -89,7 +97,7 @@ def test_create_fails(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="12345"
+                    mapping_id="mapping_id"
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -129,6 +137,9 @@ def test_create_fails_for_invalid_mapping_id(self):
                           "message": "unrecognized mapping_id"})
 
     def test_retrieve(self):
+        self._app.container_manager.container_from_url_id = mock_coro_factory(
+            DockerContainer()
+        )
         res = self.fetch("/user/username/api/v1/containers/found/",
                          headers={
                              "Cookie": "jupyter-hub-token-username=foo"
@@ -148,6 +159,9 @@ def test_retrieve(self):
         self.assertEqual(res.code, httpstatus.NOT_FOUND)
 
     def test_delete(self):
+        self._app.container_manager.container_from_url_id = mock_coro_factory(
+            DockerContainer()
+        )
         res = self.fetch("/user/username/api/v1/containers/found/",
                          method="DELETE",
                          headers={
@@ -170,6 +184,8 @@ def test_post_start(self):
                    ".container"
                    ".wait_for_http_server_2xx",
                    new_callable=mock_coro_factory):
+            self._app.container_manager.containers_from_mapping_id = \
+                mock_coro_factory(return_value=[DockerContainer()])
 
             self.assertFalse(self._app.reverse_proxy.register.called)
             self.fetch("/user/username/api/v1/containers/",
@@ -177,7 +193,7 @@ def test_post_start(self):
                        headers={
                                 "Cookie": "jupyter-hub-token-username=foo"
                        },
-                       body=escape.json_encode({"mapping_id": "12345"}))
+                       body=escape.json_encode({"mapping_id": "mapping_id"}))
 
             self.assertTrue(self._app.reverse_proxy.register.called)
 
@@ -194,6 +210,9 @@ def test_post_failed_auth(self):
         self.assertGreaterEqual(res.code, 400)
 
     def test_stop(self):
+        self._app.container_manager.container_from_url_id = mock_coro_factory(
+            DockerContainer()
+        )
         self.fetch("/user/username/api/v1/containers/12345/",
                    method="DELETE",
                    headers={

From 869230a9ef35520c2c7f2af774c51d207697b4f8 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Thu, 28 Jul 2016 17:45:36 +0100
Subject: [PATCH 25/25] Added tests for except conditions

---
 remoteappmanager/restresources/container.py | 40 +++++++-------
 tests/restmodel/test_container.py           | 61 ++++++++++++++++++++-
 2 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 191c95234..90299cb9c 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -1,4 +1,3 @@
-import contextlib
 import os
 from datetime import timedelta
 
@@ -45,9 +44,9 @@ def create(self, representation):
             raise exceptions.Unable(message=str(e))
 
         try:
-            with self._remove_container_on_error(container):
                 yield self._wait_for_container_ready(container)
         except Exception as e:
+            self._remove_container_noexcept(container)
             raise exceptions.Unable(message=str(e))
 
         urlpath = url_path_join(
@@ -55,10 +54,10 @@ def create(self, representation):
             container.urlpath)
 
         try:
-            with self._remove_container_on_error(container):
-                yield self.application.reverse_proxy.register(
-                    urlpath, container.host_url)
+            yield self.application.reverse_proxy.register(
+                urlpath, container.host_url)
         except Exception as e:
+            self._remove_container_noexcept(container)
             raise exceptions.Unable(message=str(e))
 
         return container.url_id
@@ -143,24 +142,23 @@ def items(self):
     ##################
     # Private
 
-    @contextlib.contextmanager
-    def _remove_container_on_error(self, container):
-        """Context manager that guarantees we remove the container
-        if something goes wrong during the context-held operation"""
+    @gen.coroutine
+    def _remove_container_noexcept(self, container):
+        """Removes container and silences (but logs) all exceptions
+        during this circumstance."""
+
+        # Note, can't use a context manager to perform this, because
+        # context managers are only allowed to yield once
         container_manager = self.application.container_manager
         try:
-            yield
-        except Exception as e:
-            try:
-                yield container_manager.stop_and_remove_container(
-                    container.docker_id)
-            except Exception:
-                self.log.exception(
-                    "Unable to stop container {} after "
-                    " failure to obtain a ready "
-                    "container".format(
-                        container.docker_id))
-            raise e
+            yield container_manager.stop_and_remove_container(
+                container.docker_id)
+        except Exception:
+            self.log.exception(
+                "Unable to stop container {} after "
+                " failure to obtain a ready "
+                "container".format(
+                    container.docker_id))
 
     @gen.coroutine
     def _start_container(self, user_name, app, policy, mapping_id):
diff --git a/tests/restmodel/test_container.py b/tests/restmodel/test_container.py
index aa0d26204..97d2bccb7 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -86,7 +86,7 @@ def test_create_fails(self):
                    ".container"
                    ".wait_for_http_server_2xx",
                    new_callable=mock_coro_new_callable(
-                       side_effect=TimeoutError())):
+                       side_effect=TimeoutError("timeout"))):
 
             self._app.container_manager.stop_and_remove_container = \
                 mock_coro_factory()
@@ -103,6 +103,65 @@ def test_create_fails(self):
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
             self.assertTrue(
                 self._app.container_manager.stop_and_remove_container.called)
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "Unable",
+                "message": "timeout"})
+
+    def test_create_fails_for_reverse_proxy_failure(self):
+        with patch("remoteappmanager"
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_new_callable()):
+
+            self._app.container_manager.stop_and_remove_container = \
+                mock_coro_factory()
+            self._app.reverse_proxy.register = mock_coro_factory(
+                side_effect=Exception("Boom!"))
+
+            res = self.fetch(
+                "/user/username/api/v1/containers/",
+                method="POST",
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
+                body=escape.json_encode(dict(
+                    mapping_id="mapping_id"
+                )))
+
+            self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
+            self.assertTrue(
+                self._app.container_manager.stop_and_remove_container.called)
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "Unable",
+                "message": "Boom!"})
+
+    def test_create_fails_for_start_container_failure(self):
+        with patch("remoteappmanager"
+                   ".restresources"
+                   ".container"
+                   ".wait_for_http_server_2xx",
+                   new_callable=mock_coro_new_callable()):
+
+            self._app.container_manager.stop_and_remove_container = \
+                mock_coro_factory()
+            self._app.container_manager.start_container = mock_coro_factory(
+                side_effect=Exception("Boom!"))
+
+            res = self.fetch(
+                "/user/username/api/v1/containers/",
+                method="POST",
+                headers={
+                    "Cookie": "jupyter-hub-token-username=foo"
+                },
+                body=escape.json_encode(dict(
+                    mapping_id="mapping_id"
+                )))
+
+            self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "Unable",
+                "message": "Boom!"})
 
     def test_create_fails_for_missing_mapping_id(self):
         res = self.fetch(