Remove deprecated container manager methods

remoteappmanager/docker/container_manager.py

@@ -14,8 +14,8 @@
 from remoteappmanager.logging.logging_mixin import LoggingMixin
 from remoteappmanager.utils import (
     url_path_join,
-    without_end_slash,
-    deprecated)
+    without_end_slash)
+
 
 from tornado import gen
 from traitlets import (
@@ -168,42 +168,6 @@ def stop_and_remove_container(self, container_id):
         finally:
             self._stop_pending.remove(container_id)
 
-    @gen.coroutine
-    @deprecated
-    def containers_from_mapping_id(self, user_name, mapping_id):
-        """Returns the currently running containers for a given user and
-        mapping_id.
-
-        Parameters
-        ----------
-        user_name: str
-            The username
-        mapping_id : str
-            The unique id to identify the container
-
-        Return
-        ------
-        A list of Container objects, or an empty list if nothing is found.
-        """
-        return (yield self.find_containers(user_name=user_name,
-                                           mapping_id=mapping_id))
-
-    @gen.coroutine
-    @deprecated
-    def container_from_url_id(self, url_id):
-        """Retrieves and returns the container by its url_id, if present.
-        If not present, returns None.
-        """
-        return (yield self.find_container(url_id=url_id))
-
-    @gen.coroutine
-    @deprecated
-    def running_containers(self):
-        """Returns all the running containers"""
-        containers = yield self.find_containers()
-
-        return containers
-
     @gen.coroutine
     def containers_with_labels(self, labels):
         filters = {
@@ -344,14 +308,10 @@ def _start_container(self,
         self.log.info('Got container image: {}'.format(image_name))
 
         # Check if the container is present.
-        containers = yield self.containers_from_mapping_id(
-            user_name, mapping_id)
-
-        if len(containers) != 0:
-            # we assume only one present. The API is made for potential
-            # of multiple instances.
-            container = containers[0]
+        container = yield self.find_container(
+            user_name=user_name, mapping_id=mapping_id)
 
+        if container is not None:
             # Make sure we stop and remove it if by any chance is already
             # there. This will guarantee a fresh start every time.
             self.log.info('Container for image {} '

remoteappmanager/docker/tests/test_container_manager.py

@@ -56,10 +56,12 @@ def test_start_stop(self):
         self.assertTrue(mock_client.remove_container.called)
 
     @gen_test
-    def test_containers_from_mapping_id(self):
+    def test_find_from_mapping_id(self):
         """ Test containers_for_mapping_id returns a list of Container """
-        result = yield self.manager.containers_from_mapping_id("username",
-                                                               "mapping_id")
+        result = yield self.manager.find_containers(
+            user_name="username",
+            mapping_id="mapping_id")
+
         expected = Container(docker_id='container_id1',
                              mapping_id="mapping_id",
                              name='/myrealm-username-mapping_5Fid',
@@ -76,9 +78,10 @@ def test_containers_from_mapping_id(self):
         utils.assert_containers_equal(self, result[0], expected)
 
     @gen_test
-    def test_containers_from_url_id(self):
+    def test_find_from_url_id(self):
         """ Test containers_for_mapping_id returns a list of Container """
-        result = yield self.manager.container_from_url_id("url_id")
+        result = yield self.manager.find_container(
+            url_id="url_id")
         expected = Container(docker_id='container_id1',
                              mapping_id="mapping_id",
                              name='/myrealm-username-mapping_5Fid',
@@ -95,25 +98,25 @@ def test_containers_from_url_id(self):
         utils.assert_containers_equal(self, result, expected)
 
     @gen_test
-    def test_containers_from_url_id_exceptions(self):
+    def test_find_from_url_id_exceptions(self):
         """ Test containers_for_mapping_id returns a list of Container """
         docker_client = self.mock_docker_client
         docker_client.port = mock.Mock(side_effect=Exception("Boom!"))
 
-        result = yield self.manager.container_from_url_id("url_id")
+        result = yield self.manager.find_container(url_id="url_id")
         self.assertEqual(result, None)
 
         # Making it so that no valid dictionary is returned.
         docker_client.port = mock.Mock(return_value=1234)
         self.mock_docker_client = docker_client
         self.manager._docker_client._sync_client = docker_client
 
-        result = yield self.manager.container_from_url_id("url_id")
+        result = yield self.manager.find_container(url_id="url_id")
         self.assertEqual(result, None)
 
     @gen_test
-    def test_running_containers(self):
-        result = yield self.manager.running_containers()
+    def test_find_containers(self):
+        result = yield self.manager.find_containers()
         self.assertEqual(len(result), 1)
 
     @gen_test
@@ -289,11 +292,11 @@ def test_different_realm(self):
                                    realm="anotherrealm")
         manager._docker_client._sync_client = create_docker_client()
 
-        result = yield manager.containers_from_mapping_id("user_name",
-                                                          "mapping_id")
-        self.assertEqual(result, [])
+        result = yield manager.find_container(
+            user_name="user_name", mapping_id="mapping_id")
+        self.assertIsNone(result)
 
-        result = yield manager.running_containers()
+        result = yield manager.find_containers()
         self.assertEqual(result, [])
 
     @gen_test

remoteappmanager/handlers/admin/containers_handler.py

@@ -10,7 +10,7 @@ class ContainersHandler(BaseHandler):
     @gen.coroutine
     def get(self):
         manager = self.application.container_manager
-        containers = (yield manager.running_containers())
+        containers = (yield manager.find_containers())
 
         self.render('admin/containers.html',
                     containers=containers,

remoteappmanager/tests/utils.py

@@ -116,6 +116,7 @@ def mock_coro_factory(return_value=None, side_effect=None):
     @gen.coroutine
     def coro(*args, **kwargs):
         coro.called = True
+        coro.call_args = (args, kwargs)
         yield gen.sleep(0.1)
         if side_effect:
             if isinstance(side_effect, Exception):

remoteappmanager/webapi/admin/container.py

@@ -12,7 +12,7 @@ class Container(Resource):
     def delete(self, identifier):
         """Stop the container."""
         container_manager = self.application.container_manager
-        container = yield container_manager.container_from_url_id(identifier)
+        container = yield container_manager.find_container(url_id=identifier)
 
         if not container:
             self.log.warning("Could not find container for id {}".format(

remoteappmanager/webapi/admin/tests/test_container.py

@@ -18,21 +18,21 @@ def get_app(self):
         return app
 
     def test_delete(self):
-        self._app.container_manager.container_from_url_id = mock_coro_factory(
+        self._app.container_manager.find_container = mock_coro_factory(
             DockerContainer(user="username")
         )
         self.delete("/user/username/api/v1/containers/found/",
                     httpstatus.NO_CONTENT)
 
         self.assertTrue(self._app.reverse_proxy.unregister.called)
 
-        self._app.container_manager.container_from_url_id = \
+        self._app.container_manager.find_container = \
             mock_coro_factory(return_value=None)
         self.delete("/user/username/api/v1/containers/notfound/",
                     httpstatus.NOT_FOUND)
 
     def test_delete_failure_reverse_proxy(self):
-        self._app.container_manager.container_from_url_id = mock_coro_factory(
+        self._app.container_manager.find_container = mock_coro_factory(
             DockerContainer(user="username")
         )
         self._app.reverse_proxy.unregister.side_effect = Exception()
@@ -42,7 +42,7 @@ def test_delete_failure_reverse_proxy(self):
 
     def test_delete_failure_stop_container(self):
         manager = self._app.container_manager
-        manager.container_from_url_id = mock_coro_factory(
+        manager.find_container = mock_coro_factory(
             DockerContainer(user="username")
         )
         manager.stop_and_remove_container = mock_coro_factory(

remoteappmanager/webapi/application.py

@@ -29,9 +29,9 @@ def retrieve(self, identifier):
             # available in docker.
             raise NotFound()
 
-        containers = yield container_manager.containers_from_mapping_id(
-            self.current_user.name,
-            identifier)
+        containers = yield container_manager.find_containers(
+            user_name=self.current_user.name,
+            mapping_id=identifier)
 
         representation = {
             "image": {

remoteappmanager/webapi/container.py

@@ -89,16 +89,15 @@ def create(self, representation):
     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 container_manager.find_container(
+            url_id=identifier,
+            user_name=self.current_user.name)
 
         if container is None:
             self.log.warning("Could not find container for id {}".format(
                 identifier))
             raise exceptions.NotFound()
 
-        if container.user != self.current_user.name:
-            raise exceptions.NotFound()
-
         return dict(
             name=container.name,
             image_name=container.image_name
@@ -109,16 +108,16 @@ def retrieve(self, identifier):
     def delete(self, identifier):
         """Stop the container."""
         container_manager = self.application.container_manager
-        container = yield container_manager.container_from_url_id(identifier)
+        container = yield container_manager.find_container(
+            url_id=identifier,
+            user_name=self.current_user.name
+        )
 
         if not container:
             self.log.warning("Could not find container for id {}".format(
                              identifier))
             raise exceptions.NotFound()
 
-        if container.user != self.current_user.name:
-            raise exceptions.NotFound()
-
         try:
             yield self.application.reverse_proxy.unregister(
                 container.urlpath
@@ -156,15 +155,11 @@ def items(self):
                 # available in docker. We just move on.
                 continue
 
-            containers = yield container_manager.containers_from_mapping_id(
-                self.current_user.name,
-                mapping_id)
+            container = yield container_manager.find_container(
+                user_name=self.current_user.name,
+                mapping_id=mapping_id)
 
-            # We assume that we can only run one container only (although the
-            # API considers a broader possibility for future extension.
-            if len(containers):
-                container = containers[0]
-                running_containers.append(container.url_id)
+            running_containers.append(container.url_id)
 
         return running_containers
 

remoteappmanager/webapi/tests/test_application.py

@@ -38,7 +38,7 @@ def get_app(self):
         app.container_manager = Mock()
         app.container_manager.image = mock_coro_factory(
             return_value=Image(name="boo", ui_name="foo_ui"))
-        app.container_manager.containers_from_mapping_id = mock_coro_factory(
+        app.container_manager.find_containers = mock_coro_factory(
             return_value=[])
         application_mock_1 = Mock()
         application_mock_1.image = "hello1"
@@ -94,7 +94,7 @@ def test_retrieve(self):
                           },
                           'mapping_id': 'one'})
 
-        self._app.container_manager.containers_from_mapping_id = \
+        self._app.container_manager.find_containers = \
             mock_coro_factory(return_value=[Container(
                 name="container",
                 image_name="xxx",

remoteappmanager/webapi/tests/test_container.py

@@ -25,7 +25,7 @@ def get_app(self):
     def test_items(self):
         manager = self._app.container_manager
         manager.image = mock_coro_factory(Image())
-        manager.containers_from_mapping_id = mock_coro_factory(
+        manager.find_containers = mock_coro_factory(
             [DockerContainer()])
 
         code, data = self.get(
@@ -221,7 +221,7 @@ 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(
+        self._app.container_manager.find_container = mock_coro_factory(
             DockerContainer(user="username")
         )
         _, data = self.get("/user/username/api/v1/containers/found/",
@@ -230,47 +230,50 @@ def test_retrieve(self):
         self.assertEqual(data["image_name"], "")
         self.assertEqual(data["name"], "")
 
-        self._app.container_manager.container_from_url_id = \
+        self._app.container_manager.find_container = \
             mock_coro_factory(return_value=None)
         self.get("/user/username/api/v1/containers/notfound/",
                  httpstatus.NOT_FOUND)
 
     def test_prevent_retrieve_from_other_user(self):
-        self._app.container_manager.container_from_url_id = mock_coro_factory(
-            DockerContainer(user="foo")
-        )
+        self._app.container_manager.find_container = mock_coro_factory(None)
 
         self.get("/user/username/api/v1/containers/found/",
                  httpstatus.NOT_FOUND)
+        kwargs = self._app.container_manager.find_container.call_args[1]
+        self.assertEqual(kwargs["user_name"], "username")
 
     def test_delete(self):
-        self._app.container_manager.container_from_url_id = mock_coro_factory(
+        self._app.container_manager.find_container = mock_coro_factory(
             DockerContainer(user="username")
         )
 
         self.delete("/user/username/api/v1/containers/found/",
                     httpstatus.NO_CONTENT)
         self.assertTrue(self._app.reverse_proxy.unregister.called)
 
-        self._app.container_manager.container_from_url_id = \
+        self._app.container_manager.find_container = \
             mock_coro_factory(return_value=None)
         self.delete("/user/username/api/v1/containers/notfound/",
                     httpstatus.NOT_FOUND)
 
     def test_prevent_delete_from_other_user(self):
-        self._app.container_manager.container_from_url_id = mock_coro_factory(
-            DockerContainer(user="foo")
+        self._app.container_manager.find_container = mock_coro_factory(
+            None
         )
         self.delete("/user/username/api/v1/containers/found/",
                     httpstatus.NOT_FOUND)
 
+        kwargs = self._app.container_manager.find_container.call_args[1]
+        self.assertEqual(kwargs["user_name"], "username")
+
     def test_post_start(self):
         with patch("remoteappmanager"
                    ".webapi"
                    ".container"
                    ".wait_for_http_server_2xx",
                    new_callable=mock_coro_factory):
-            self._app.container_manager.containers_from_mapping_id = \
+            self._app.container_manager.find_containers = \
                 mock_coro_factory(return_value=[DockerContainer()])
 
             self.assertFalse(self._app.reverse_proxy.register.called)

selenium_tests/test_admin_name_header_bug.py

@@ -5,7 +5,6 @@ class TestAdminNameHeaderBug(SeleniumTestBase):
     def test_admin_name_header_bug(self):
         driver = self.driver
         with self.login("admin"):
-            print("hello")
             driver.find_element_by_link_text("Users").click()
             driver.find_element_by_link_text("Show").click()
             self.wait_for(lambda: