diff --git a/remoteappmanager/webapi/admin/accounting.py b/remoteappmanager/webapi/admin/accounting.py index f75c6efd6..f39a858f0 100644 --- a/remoteappmanager/webapi/admin/accounting.py +++ b/remoteappmanager/webapi/admin/accounting.py @@ -11,7 +11,7 @@ class Accounting(Resource): __collection_name__ = "accounting" - def validate(self, representation): + def validate_representation(self, representation): representation["user_name"] = _not_empty_str( representation["user_name"]) representation["image_name"] = _not_empty_str( @@ -23,6 +23,7 @@ def validate(self, representation): representation["volume"] = _not_empty_str( representation["volume"]) parse_volume_string(representation["volume"]) + return representation @gen.coroutine @authenticated @@ -44,11 +45,11 @@ def create(self, representation): @gen.coroutine @authenticated - def delete(self, id): + def delete(self, identifier): db = self.application.db try: - db.revoke_access_by_id(id) + db.revoke_access_by_id(identifier) except db_exceptions.NotFound: raise exceptions.NotFound() diff --git a/remoteappmanager/webapi/admin/application.py b/remoteappmanager/webapi/admin/application.py index 2c4a25e00..6e794774f 100644 --- a/remoteappmanager/webapi/admin/application.py +++ b/remoteappmanager/webapi/admin/application.py @@ -8,22 +8,25 @@ class Application(Resource): - def validate(self, representation): + def validate_representation(self, representation): representation["image_name"] = str(representation["image_name"]) + if len(representation["image_name"]) == 0: + raise ValueError("image_name cannot be empty") + + return representation + + def validate_identifier(self, identifier): + return int(identifier) @gen.coroutine @authenticated def delete(self, identifier): """Removes the application.""" db = self.application.db - try: - id = int(identifier) - except ValueError: - raise exceptions.BadRequest("id") try: - db.remove_application(id=id) - self.log.info("Removed application with id {}".format(id)) + db.remove_application(id=identifier) + self.log.info("Removed application with id {}".format(identifier)) except db_exceptions.NotFound: raise exceptions.NotFound() except db_exceptions.UnsupportedOperation: diff --git a/remoteappmanager/webapi/admin/tests/test_application.py b/remoteappmanager/webapi/admin/tests/test_application.py index ae195e36f..5bec261ac 100644 --- a/remoteappmanager/webapi/admin/tests/test_application.py +++ b/remoteappmanager/webapi/admin/tests/test_application.py @@ -25,7 +25,7 @@ def test_delete(self): httpstatus.NOT_FOUND) self.delete("/user/username/api/v1/applications/foo/", - httpstatus.BAD_REQUEST) + httpstatus.NOT_FOUND) def test_unable_to_delete(self): with mock.patch("remoteappmanager.tests.mocking." @@ -53,6 +53,15 @@ def test_unable_to_create(self): {"image_name": "foobar"}, httpstatus.INTERNAL_SERVER_ERROR) + def test_create_invalid_representation(self): + self.post("/user/username/api/v1/applications/", + {"image_name": ""}, + httpstatus.BAD_REQUEST) + + self.post("/user/username/api/v1/applications/", + {}, + httpstatus.BAD_REQUEST) + def test_delete_failed_auth(self): self._app.hub.verify_token.return_value = {} diff --git a/remoteappmanager/webapi/admin/tests/test_user.py b/remoteappmanager/webapi/admin/tests/test_user.py index d7d3f61a8..378bd2cdd 100644 --- a/remoteappmanager/webapi/admin/tests/test_user.py +++ b/remoteappmanager/webapi/admin/tests/test_user.py @@ -25,7 +25,7 @@ def test_delete(self): httpstatus.NOT_FOUND) self.delete("/user/username/api/v1/users/foo/", - httpstatus.BAD_REQUEST) + httpstatus.NOT_FOUND) def test_unable_to_delete(self): with mock.patch("remoteappmanager.tests.mocking." diff --git a/remoteappmanager/webapi/admin/user.py b/remoteappmanager/webapi/admin/user.py index 5609e837f..3eb7cdcbc 100644 --- a/remoteappmanager/webapi/admin/user.py +++ b/remoteappmanager/webapi/admin/user.py @@ -8,24 +8,23 @@ class User(Resource): - def validate(self, representation): + def validate_representation(self, representation): representation["name"] = str(representation["name"]).strip() if len(representation["name"]) == 0: raise ValueError("name cannot be empty") + return representation + + def validate_identifier(self, identifier): + return int(identifier) @gen.coroutine @authenticated def delete(self, identifier): - """Removes the user.""" db = self.application.db - try: - id = int(identifier) - except ValueError: - raise exceptions.BadRequest("id") try: - db.remove_user(id=id) - self.log.info("Removed user with id {}".format(id)) + db.remove_user(id=identifier) + self.log.info("Removed user with id {}".format(identifier)) except db_exceptions.NotFound: raise exceptions.NotFound() except db_exceptions.UnsupportedOperation: diff --git a/remoteappmanager/webapi/container.py b/remoteappmanager/webapi/container.py index 9cc56065f..73ed1c4a5 100644 --- a/remoteappmanager/webapi/container.py +++ b/remoteappmanager/webapi/container.py @@ -4,7 +4,6 @@ from tornado import gen from tornadowebapi import exceptions -from tornadowebapi.exceptions import NotFound from tornadowebapi.resource import Resource from remoteappmanager.netutils import wait_for_http_server_2xx @@ -12,19 +11,22 @@ class Container(Resource): + def validate_representation(self, representation): + try: + representation["mapping_id"] + except KeyError: + raise exceptions.BadRepresentation(message="missing mapping_id") + + return representation + @gen.coroutine @authenticated def create(self, representation): """Create the container. The representation should accept the application mapping id we want to start""" - if self.current_user is None: - raise NotFound() - try: - mapping_id = representation["mapping_id"] - except KeyError: - raise exceptions.BadRequest(message="missing mapping_id") + mapping_id = representation["mapping_id"] webapp = self.application account = self.current_user.account @@ -38,19 +40,20 @@ def create(self, representation): if not choice: self.log.warning("Could not find resource " "for mapping id {}".format(mapping_id)) - raise exceptions.BadRequest(message="unrecognized mapping_id") + raise exceptions.BadRepresentation( + message="unrecognized mapping_id") _, app, policy = choice[0] image = yield container_manager.image(app.image) if image is None: - raise exceptions.BadRequest(message="unrecognized image") + raise exceptions.BadRepresentation(message="unrecognized image") try: environment = self._environment_from_configurables(image, representation) except Exception: - raise exceptions.BadRequest(message="invalid configurables") + raise exceptions.BadRepresentation(message="invalid configurables") # Everything is fine. Start and wait for the container to come online. try: diff --git a/remoteappmanager/webapi/tests/test_container.py b/remoteappmanager/webapi/tests/test_container.py index e0c6c3e2f..69056cad9 100644 --- a/remoteappmanager/webapi/tests/test_container.py +++ b/remoteappmanager/webapi/tests/test_container.py @@ -206,7 +206,7 @@ def test_create_fails_for_missing_mapping_id(self): ) self.assertEqual(data, - {"type": "BadRequest", + {"type": "BadRepresentation", "message": "missing mapping_id"}) def test_create_fails_for_invalid_mapping_id(self): @@ -217,7 +217,7 @@ def test_create_fails_for_invalid_mapping_id(self): ) self.assertEqual(data, - {"type": "BadRequest", + {"type": "BadRepresentation", "message": "unrecognized mapping_id"}) def test_retrieve(self): diff --git a/requirements.txt b/requirements.txt index c6a6b5c46..f2ca9f4aa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,4 +12,4 @@ git+git://github.com/jupyterhub/jupyterhub.git@42a993fd084780ad23e475f27f67ade3a jupyter_client==4.3.0 click==6.6 tabulate==0.7.5 -git+git://github.com/simphony/tornado-webapi.git@v0.4.2#egg=tornadowebapi +git+git://github.com/simphony/tornado-webapi.git#egg=tornadowebapi diff --git a/setup.py b/setup.py index 56765bf43..a302cf94d 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ "jupyter_client>=4.3.0", "click>=6.6", "tabulate>=0.7.5", - "tornadowebapi>=0.4.2" + "tornadowebapi>=0.5.0.dev0" ] # Unfortunately RTD cannot install jupyterhub because jupyterhub needs bower,