Repository: simphony/simphony-remote

Issue:
Introduce REST error payload delivery
The current design of the REST handler does not deliver error payload.
This is needed now because we want to report REST exceptions with more detail. 


Discussion:

Patch:
diff --git a/doc/Makefile b/doc/Makefile
index 580483d..22a433c 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.rest.http.rst b/doc/source/api/remoteappmanager.rest.http.rst
new file mode 100644
index 0000000..d045864
--- /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 a84fc02..5615c69 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 84edd77..8db8b7d 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
 -----------------------------
 
diff --git a/remoteappmanager/rest/exceptions.py b/remoteappmanager/rest/exceptions.py
index 4be58ac..6ccc584 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,26 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
+    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):
+        """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)
+
+        return data
+
 
 class NotFound(RESTException):
     """Exception raised when the resource is not found.
@@ -17,6 +37,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
@@ -27,5 +52,8 @@ class BadRequest(RESTException):
     http_code = httpstatus.BAD_REQUEST
 
 
-class InternalServerError(RESTException):
-    pass
+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/rest/http/__init__.py b/remoteappmanager/rest/http/__init__.py
new file mode 100644
index 0000000..e69de29
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 0000000..dd37b1e
--- /dev/null
+++ b/remoteappmanager/rest/http/payloaded_http_error.py
@@ -0,0 +1,36 @@
+from tornado.web import HTTPError
+
+
+class PayloadedHTTPError(HTTPError):
+    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:
+            if not isinstance(payload, str):
+                raise ValueError("payload must be a string.")
+
+            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/remoteappmanager/rest/rest_handler.py b/remoteappmanager/rest/rest_handler.py
index 77a65aa..78a97c9 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,44 @@ class RESTBaseHandler(BaseHandler):
         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.clear_header('Content-Type')
+            self.finish()
+
+        exc = exc_info[1]
+
+        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
+            # 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):
+        """Converts a REST exception into the appropriate HTTP one."""
+
+        representation = rest_exc.representation()
+        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=content_type
+        )
+
 
 class RESTCollectionHandler(RESTBaseHandler):
     """Handler for URLs addressing a collection.
@@ -38,7 +77,7 @@ class RESTCollectionHandler(RESTBaseHandler):
         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 +106,7 @@ class RESTCollectionHandler(RESTBaseHandler):
         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:
@@ -87,6 +126,7 @@ class RESTCollectionHandler(RESTBaseHandler):
 
         self.set_status(httpstatus.CREATED)
         self.set_header("Location", location)
+        self.clear_header('Content-Type')
         self.flush()
 
 
@@ -104,7 +144,7 @@ class RESTResourceHandler(RESTBaseHandler):
         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 +168,8 @@ class RESTResourceHandler(RESTBaseHandler):
 
         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 +198,7 @@ class RESTResourceHandler(RESTBaseHandler):
         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:
@@ -166,6 +208,7 @@ class RESTResourceHandler(RESTBaseHandler):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)
 
     @web.authenticated
@@ -176,7 +219,7 @@ class RESTResourceHandler(RESTBaseHandler):
         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:
@@ -186,4 +229,5 @@ class RESTResourceHandler(RESTBaseHandler):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)
diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 5ad389e..90299cb 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -3,10 +3,8 @@ from datetime import timedelta
 
 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
 
@@ -17,11 +15,13 @@ class Container(Resource):
         """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(message="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 +30,9 @@ class Container(Resource):
         if not choice:
             self.log.warning("Could not find resource "
                              "for mapping id {}".format(mapping_id))
-            raise exceptions.BadRequest()
+            raise exceptions.BadRequest(message="unrecognized mapping_id")
 
         _, app, policy = choice[0]
-        container = None
 
         try:
             container = yield self._start_container(
@@ -41,33 +40,33 @@ class Container(Resource):
                 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.InternalServerError()
+            raise exceptions.Unable(message=str(e))
+
+        try:
+                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(
             self.application.command_line_config.base_urlpath,
             container.urlpath)
-        yield self.application.reverse_proxy.register(urlpath,
-                                                      container.host_url)
+
+        try:
+            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
 
     @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(
@@ -82,7 +81,9 @@ class Container(Resource):
     @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(
                              identifier))
@@ -91,9 +92,22 @@ class Container(Resource):
         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,24 +139,26 @@ class Container(Resource):
 
         return running_containers
 
-    @gen.coroutine
-    def _container_from_url_id(self, container_url_id):
-        """Retrieves and returns the container if valid and present.
+    ##################
+    # Private
 
-        If not present, returns None
-        """
+    @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
-
-        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])
+        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))
 
     @gen.coroutine
     def _start_container(self, user_name, app, policy, mapping_id):
diff --git a/remoteappmanager/static/js/remoteappapi.js b/remoteappmanager/static/js/remoteappapi.js
index 3de176a..f7fdb51 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
@@ -46,7 +46,6 @@ define(['jquery', 'utils'], function ($, utils) {
         options = options || {};
         options = update(options, {
             type: 'POST', 
-            dataType: null,
             data: JSON.stringify({
                 mapping_id: id
             })});
@@ -58,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


Test patch:
diff --git a/tests/rest/http/__init__.py b/tests/rest/http/__init__.py
new file mode 100644
index 0000000..e69de29
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 0000000..94e4dfa
--- /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)
diff --git a/tests/rest/test_rest.py b/tests/rest/test_rest.py
index db15d58..5e45b51 100644
--- a/tests/rest/test_rest.py
+++ b/tests/rest/test_rest.py
@@ -1,18 +1,17 @@
 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, exceptions
+from remoteappmanager.rest.http import httpstatus
 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):
@@ -50,7 +49,7 @@ class Student(Resource):
     @gen.coroutine
     def delete(self, identifier):
         if identifier not in self.collection:
-            raise exceptions.NotFound
+            raise exceptions.NotFound()
 
         del self.collection[identifier]
 
@@ -66,11 +65,19 @@ 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")
+
+    @gen.coroutine
+    def items(self):
+        raise exceptions.BadRequest("unprocessable", foo="bar")
 
 
 class UnsupportsCollection(Resource):
@@ -183,6 +190,7 @@ class TestREST(AsyncHTTPTestCase):
 
             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"
@@ -349,6 +357,24 @@ class TestREST(AsyncHTTPTestCase):
                 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/",
+                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/",
@@ -356,6 +382,37 @@ class TestREST(AsyncHTTPTestCase):
                 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/"
diff --git a/tests/restmodel/test_application.py b/tests/restmodel/test_application.py
index a08a816..a4fd9de 100644
--- a/tests/restmodel/test_application.py
+++ b/tests/restmodel/test_application.py
@@ -1,13 +1,12 @@
 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 Application
 from tests import utils
-from tornado import web, escape
-
-from remoteappmanager import rest
-from remoteappmanager.rest import registry, httpstatus
-
 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 e125b13..97d2bcc 100644
--- a/tests/restmodel/test_container.py
+++ b/tests/restmodel/test_container.py
@@ -1,115 +1,281 @@
-from unittest.mock import Mock, patch
-
-from tornado import escape
-
-from remoteappmanager.rest import httpstatus
+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
+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 TestContainer(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 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 prepare_side_effect(*args, **kwargs):
-            user = Mock()
-            user.name = 'user_name'
-            args[0].current_user = user
+        self.addCleanup(cleanup)
 
-        self.mock_prepare = mock_coro_new_callable(
-            side_effect=prepare_side_effect)
+        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"]})
+        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={
+                "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",  # 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"):
+        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="mapping_id"
+                )))
 
             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("timeout"))):
 
+            self._app.container_manager.stop_and_remove_container = \
+                mock_coro_factory()
             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"
                 )))
 
             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)
+            self.assertEqual(escape.json_decode(res.body), {
+                "type": "Unable",
+                "message": "timeout"})
 
-    def test_retrieve(self):
-        with patch("remoteappmanager.handlers.base_handler.BaseHandler.prepare",  # noqa
-                   new_callable=self.mock_prepare):
+    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"
+                )))
 
-            res = self.fetch("/api/v1/containers/notfound/")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
+            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!"})
 
-            res = self.fetch("/api/v1/containers/url_id/")
-            self.assertEqual(res.code, httpstatus.OK)
+    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()):
 
-            content = escape.json_decode(res.body)
-            self.assertEqual(content["image_name"], "image_name1")
-            self.assertEqual(content["name"],
-                             "/remoteexec-username-mapping_5Fid")
+            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(
+            "/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):
+        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"
+                         })
+        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):
+        self._app.container_manager.container_from_url_id = mock_coro_factory(
+            DockerContainer()
+        )
+        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._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/",
+                       method="POST",
+                       headers={
+                                "Cookie": "jupyter-hub-token-username=foo"
+                       },
+                       body=escape.json_encode({"mapping_id": "mapping_id"}))
+
+            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)
 
-            res = self.fetch("/api/v1/containers/notfound/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NOT_FOUND)
+    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={
+                      "Cookie": "jupyter-hub-token-username=foo"
+                   })
 
-            res = self.fetch("/api/v1/containers/url_id/", method="DELETE")
-            self.assertEqual(res.code, httpstatus.NO_CONTENT)
+        self.assertTrue(self._app.reverse_proxy.unregister.called)