From 92b94b59ad80329a2c99471edbf5bbdc9af1e525 Mon Sep 17 00:00:00 2001 From: Georgios Andreadis Date: Fri, 26 Jun 2020 12:17:26 +0200 Subject: Revamp error responses --- opendc/api/v2/simulations/endpoint.py | 5 +-- opendc/api/v2/simulations/simulationId/endpoint.py | 40 +++++++--------------- .../simulationId/topologies/endpoint.py | 5 +-- opendc/api/v2/topologies/topologyId/endpoint.py | 5 +-- opendc/api/v2/users/endpoint.py | 10 ++---- opendc/api/v2/users/userId/endpoint.py | 23 ++++--------- opendc/util/exceptions.py | 8 +++++ opendc/util/rest.py | 13 +++++-- 8 files changed, 43 insertions(+), 66 deletions(-) diff --git a/opendc/api/v2/simulations/endpoint.py b/opendc/api/v2/simulations/endpoint.py index dfeb8d5e..62257d40 100644 --- a/opendc/api/v2/simulations/endpoint.py +++ b/opendc/api/v2/simulations/endpoint.py @@ -11,10 +11,7 @@ from opendc.util.rest import Response def POST(request): """Create a new simulation, and return that new simulation.""" - try: - request.check_required_parameters(body={'simulation': {'name': 'string'}}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(body={'simulation': {'name': 'string'}}) topology = Topology({'name': 'Default topology'}) topology.insert() diff --git a/opendc/api/v2/simulations/simulationId/endpoint.py b/opendc/api/v2/simulations/simulationId/endpoint.py index b08cf8be..b8ae9a38 100644 --- a/opendc/api/v2/simulations/simulationId/endpoint.py +++ b/opendc/api/v2/simulations/simulationId/endpoint.py @@ -10,10 +10,7 @@ from opendc.util.rest import Response def GET(request): """Get this Simulation.""" - try: - request.check_required_parameters(path={'simulationId': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(path={'simulationId': 'string'}) simulation = Simulation.from_id(request.params_path['simulationId']) validation_error = simulation.validate() @@ -30,10 +27,7 @@ def GET(request): def PUT(request): """Update a simulation's name.""" - try: - request.check_required_parameters(body={'simulation': {'name': 'name'}}, path={'simulationId': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(body={'simulation': {'name': 'name'}}, path={'simulationId': 'string'}) simulation = Simulation.from_id(request.params_path['simulationId']) @@ -55,30 +49,20 @@ def PUT(request): def DELETE(request): """Delete this Simulation.""" - # Make sure required parameters are there + request.check_required_parameters(path={'simulationId': 'string'}) - try: - request.check_required_parameters(path={'simulationId': 'string'}) - - except exceptions.ParameterError as e: - return Response(400, str(e)) - - # Instantiate a Simulation and make sure it exists - - simulation = Simulation.from_primary_key((request.params_path['simulationId'], )) - - if not simulation.exists(): - return Response(404, '{} not found.'.format(simulation)) + simulation = Simulation.from_id(request.params_path['simulationId']) - # Make sure this User is allowed to delete this Simulation + validation_error = simulation.validate() + if validation_error is not None: + return validation_error - if not simulation.google_id_has_at_least(request.google_id, 'OWN'): - return Response(403, 'Forbidden from deleting {}.'.format(simulation)) + access_error = simulation.validate_user_access(request.google_id, True) + if access_error is not None: + return access_error - # Delete this Simulation from the database + # FIXME cascading simulation.delete() - # Return this Simulation - - return Response(200, 'Successfully deleted {}.'.format(simulation), simulation.to_JSON()) + return Response(200, f'Successfully deleted simulation.', simulation.obj) diff --git a/opendc/api/v2/simulations/simulationId/topologies/endpoint.py b/opendc/api/v2/simulations/simulationId/topologies/endpoint.py index 4104c4dd..d8467b40 100644 --- a/opendc/api/v2/simulations/simulationId/topologies/endpoint.py +++ b/opendc/api/v2/simulations/simulationId/topologies/endpoint.py @@ -12,10 +12,7 @@ def POST(request): # Make sure required parameters are there - try: - request.check_required_parameters(body={'topology': {'name': 'string'}}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(body={'topology': {'name': 'string'}}) # Instantiate a Topology object from our request, and add some metadata diff --git a/opendc/api/v2/topologies/topologyId/endpoint.py b/opendc/api/v2/topologies/topologyId/endpoint.py index ef541daa..719048c4 100644 --- a/opendc/api/v2/topologies/topologyId/endpoint.py +++ b/opendc/api/v2/topologies/topologyId/endpoint.py @@ -8,10 +8,7 @@ def GET(request): # Make sure required parameters are there - try: - request.check_required_parameters(path={'topologyId': 'int'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(path={'topologyId': 'int'}) # Instantiate a Topology from the database diff --git a/opendc/api/v2/users/endpoint.py b/opendc/api/v2/users/endpoint.py index b1a3675d..4b0a883a 100644 --- a/opendc/api/v2/users/endpoint.py +++ b/opendc/api/v2/users/endpoint.py @@ -7,10 +7,7 @@ from opendc.util.rest import Response def GET(request): """Search for a User using their email address.""" - try: - request.check_required_parameters(query={'email': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(query={'email': 'string'}) user = User.from_email(request.params_query['email']) @@ -24,10 +21,7 @@ def GET(request): def POST(request): """Add a new User.""" - try: - request.check_required_parameters(body={'user': {'email': 'string'}}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(body={'user': {'email': 'string'}}) user = User(request.params_body['user']) user.set_property('googleId', request.google_id) diff --git a/opendc/api/v2/users/userId/endpoint.py b/opendc/api/v2/users/userId/endpoint.py index 3fb2ecc8..578080b7 100644 --- a/opendc/api/v2/users/userId/endpoint.py +++ b/opendc/api/v2/users/userId/endpoint.py @@ -6,10 +6,7 @@ from opendc.util.rest import Response def GET(request): """Get this User.""" - try: - request.check_required_parameters(path={'userId': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(path={'userId': 'string'}) user = User.from_id(request.params_path['userId']) @@ -23,14 +20,11 @@ def GET(request): def PUT(request): """Update this User's given name and/or family name.""" - try: - request.check_required_parameters(body={'user': { - 'givenName': 'string', - 'familyName': 'string' - }}, - path={'userId': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(body={'user': { + 'givenName': 'string', + 'familyName': 'string' + }}, + path={'userId': 'string'}) user = User.from_id(request.params_path['userId']) @@ -49,10 +43,7 @@ def PUT(request): def DELETE(request): """Delete this User.""" - try: - request.check_required_parameters(path={'userId': 'string'}) - except exceptions.ParameterError as e: - return Response(400, str(e)) + request.check_required_parameters(path={'userId': 'string'}) user = User.from_id(request.params_path['userId']) diff --git a/opendc/util/exceptions.py b/opendc/util/exceptions.py index caf6dd8e..e73dad4f 100644 --- a/opendc/util/exceptions.py +++ b/opendc/util/exceptions.py @@ -55,3 +55,11 @@ class MissingParameterError(ParameterError): self.parameter_name = parameter_name self.parameter_location = parameter_location + + +class ClientRequestError(Exception): + """Raised when a 4xx response is to be returned.""" + + def __init__(self, response): + super(ClientRequestError, self).__init__(str(response)) + self.response = response diff --git a/opendc/util/rest.py b/opendc/util/rest.py index 33371e52..e70998a8 100644 --- a/opendc/util/rest.py +++ b/opendc/util/rest.py @@ -6,6 +6,7 @@ import sys from oauth2client import client, crypt from opendc.util import exceptions, parameter_checker +from opendc.util.exceptions import ClientRequestError class Request(object): @@ -71,14 +72,22 @@ class Request(object): def check_required_parameters(self, **kwargs): """Raise an error if a parameter is missing or of the wrong type.""" - parameter_checker.check(self, **kwargs) + try: + parameter_checker.check(self, **kwargs) + except exceptions.ParameterError as e: + raise ClientRequestError(Response(400, str(e))) def process(self): """Process the Request and return a Response.""" method = getattr(self.module, self.method) - response = method(self) + try: + response = method(self) + except ClientRequestError as e: + e.response.id = self.id + return e.response + response.id = self.id return response -- cgit v1.2.3 From 19bede4fc7f7320bb4eb16c3fe1a211b19ab4714 Mon Sep 17 00:00:00 2001 From: Georgios Andreadis Date: Fri, 26 Jun 2020 12:27:51 +0200 Subject: Revamp error responses everywhere --- opendc/api/v2/simulations/simulationId/endpoint.py | 26 +++++----------------- opendc/api/v2/topologies/topologyId/endpoint.py | 18 ++------------- opendc/api/v2/users/endpoint.py | 8 ++----- opendc/api/v2/users/userId/endpoint.py | 14 +++++------- opendc/models/model.py | 7 +++--- opendc/models/simulation.py | 7 +++--- opendc/models/topology.py | 7 +++--- opendc/models/user.py | 18 +++++---------- opendc/util/exceptions.py | 4 ++-- opendc/util/rest.py | 6 ++--- 10 files changed, 34 insertions(+), 81 deletions(-) diff --git a/opendc/api/v2/simulations/simulationId/endpoint.py b/opendc/api/v2/simulations/simulationId/endpoint.py index b8ae9a38..8d29202d 100644 --- a/opendc/api/v2/simulations/simulationId/endpoint.py +++ b/opendc/api/v2/simulations/simulationId/endpoint.py @@ -13,13 +13,9 @@ def GET(request): request.check_required_parameters(path={'simulationId': 'string'}) simulation = Simulation.from_id(request.params_path['simulationId']) - validation_error = simulation.validate() - if validation_error is not None: - return validation_error - access_error = simulation.validate_user_access(request.google_id, False) - if access_error is not None: - return access_error + simulation.check_exists() + simulation.check_user_access(request.google_id, False) return Response(200, 'Successfully retrieved simulation', simulation.obj) @@ -31,13 +27,8 @@ def PUT(request): simulation = Simulation.from_id(request.params_path['simulationId']) - validation_error = simulation.validate() - if validation_error is not None: - return validation_error - - access_error = simulation.validate_user_access(request.google_id, True) - if access_error is not None: - return access_error + simulation.check_exists() + simulation.check_user_access(request.google_id, True) simulation.set_property('name', request.params_body['simulation']['name']) simulation.set_property('datetime_last_edited', Database.datetime_to_string(datetime.now())) @@ -53,13 +44,8 @@ def DELETE(request): simulation = Simulation.from_id(request.params_path['simulationId']) - validation_error = simulation.validate() - if validation_error is not None: - return validation_error - - access_error = simulation.validate_user_access(request.google_id, True) - if access_error is not None: - return access_error + simulation.check_exists() + simulation.check_user_access(request.google_id, True) # FIXME cascading diff --git a/opendc/api/v2/topologies/topologyId/endpoint.py b/opendc/api/v2/topologies/topologyId/endpoint.py index 719048c4..3470ad94 100644 --- a/opendc/api/v2/topologies/topologyId/endpoint.py +++ b/opendc/api/v2/topologies/topologyId/endpoint.py @@ -6,26 +6,12 @@ from opendc.util.rest import Response def GET(request): """Get this Topology.""" - # Make sure required parameters are there - request.check_required_parameters(path={'topologyId': 'int'}) - # Instantiate a Topology from the database - topology = Topology.from_id(request.params_path['topologyId']) - # Make sure this Topology exists - - validation_error = topology.validate() - if validation_error is not None: - return validation_error - - # Make sure this user is authorized to view this Topology - - access_error = topology.validate_user_access(request.google_id, False) - if access_error is not None: - return access_error + topology.check_exists() - # Return this Topology + topology.check_user_access(request.google_id, False) return Response(200, 'Successfully retrieved topology.', topology.obj) diff --git a/opendc/api/v2/users/endpoint.py b/opendc/api/v2/users/endpoint.py index 4b0a883a..c6041756 100644 --- a/opendc/api/v2/users/endpoint.py +++ b/opendc/api/v2/users/endpoint.py @@ -11,9 +11,7 @@ def GET(request): user = User.from_email(request.params_query['email']) - validation_error = user.validate() - if validation_error is not None: - return validation_error + user.check_exists() return Response(200, f'Successfully retrieved user.', user.obj) @@ -27,9 +25,7 @@ def POST(request): user.set_property('googleId', request.google_id) user.set_property('authorizations', []) - validation_error = user.validate_insertion() - if validation_error is not None: - return validation_error + user.check_already_exists() user.insert() return Response(200, f'Successfully created user.', user.obj) diff --git a/opendc/api/v2/users/userId/endpoint.py b/opendc/api/v2/users/userId/endpoint.py index 578080b7..e68a2bb3 100644 --- a/opendc/api/v2/users/userId/endpoint.py +++ b/opendc/api/v2/users/userId/endpoint.py @@ -10,9 +10,7 @@ def GET(request): user = User.from_id(request.params_path['userId']) - validation_error = user.validate() - if validation_error is not None: - return validation_error + user.check_exists() return Response(200, f'Successfully retrieved user.', user.obj) @@ -28,9 +26,8 @@ def PUT(request): user = User.from_id(request.params_path['userId']) - validation_error = user.validate(request.google_id) - if validation_error is not None: - return validation_error + user.check_exists() + user.check_correct_user(request.google_id) user.set_property('givenName', request.params_body['user']['givenName']) user.set_property('familyName', request.params_body['user']['familyName']) @@ -47,9 +44,8 @@ def DELETE(request): user = User.from_id(request.params_path['userId']) - validation_error = user.validate(request.google_id) - if validation_error is not None: - return validation_error + user.check_exists() + user.check_correct_user(request.google_id) user.delete() diff --git a/opendc/models/model.py b/opendc/models/model.py index d0bf34ee..2505ae61 100644 --- a/opendc/models/model.py +++ b/opendc/models/model.py @@ -1,4 +1,5 @@ from opendc.util.database import DB +from opendc.util.exceptions import ClientError from opendc.util.rest import Response @@ -12,11 +13,9 @@ class Model: def __init__(self, obj): self.obj = obj - def validate(self, request_google_id=None): + def check_exists(self): if self.obj is None: - return Response(404, 'Not found.') - - return None + raise ClientError(Response(404, 'Not found.')) def set_property(self, key, value): self.obj[key] = value diff --git a/opendc/models/simulation.py b/opendc/models/simulation.py index f58581cf..5cd3d49e 100644 --- a/opendc/models/simulation.py +++ b/opendc/models/simulation.py @@ -1,16 +1,15 @@ from opendc.models.model import Model from opendc.models.user import User +from opendc.util.exceptions import ClientError from opendc.util.rest import Response class Simulation(Model): collection_name = 'simulations' - def validate_user_access(self, google_id, edit_access): + def check_user_access(self, google_id, edit_access): user = User.from_google_id(google_id) authorizations = list( filter(lambda x: str(x['simulationId']) == str(self.obj['_id']), user.obj['authorizations'])) if len(authorizations) == 0 or (edit_access and authorizations[0]['authorizationLevel'] == 'VIEW'): - return Response(403, "Forbidden from retrieving simulation.") - - return None + raise ClientError(Response(403, "Forbidden from retrieving simulation.")) diff --git a/opendc/models/topology.py b/opendc/models/topology.py index b01d5f41..6dde3e2a 100644 --- a/opendc/models/topology.py +++ b/opendc/models/topology.py @@ -1,16 +1,15 @@ from opendc.models.model import Model from opendc.models.user import User +from opendc.util.exceptions import ClientError from opendc.util.rest import Response class Topology(Model): collection_name = 'topologies' - def validate_user_access(self, google_id, edit_access): + def check_user_access(self, google_id, edit_access): user = User.from_google_id(google_id) authorizations = list( filter(lambda x: str(x['topologyId']) == str(self.obj['_id']), user.obj['authorizations'])) if len(authorizations) == 0 or (edit_access and authorizations[0]['authorizationLevel'] == 'VIEW'): - return Response(403, "Forbidden from retrieving topology.") - - return None \ No newline at end of file + raise ClientError(Response(403, "Forbidden from retrieving topology.")) diff --git a/opendc/models/user.py b/opendc/models/user.py index ea8b1f3f..cd314457 100644 --- a/opendc/models/user.py +++ b/opendc/models/user.py @@ -1,5 +1,6 @@ from opendc.models.model import Model from opendc.util.database import DB +from opendc.util.exceptions import ClientError from opendc.util.rest import Response @@ -14,21 +15,12 @@ class User(Model): def from_google_id(cls, google_id): return User(DB.fetch_one({'googleId': google_id}, User.collection_name)) - def validate(self, request_google_id=None): - super_validation = super().validate(request_google_id) - - if super_validation is not None: - return super_validation - + def check_correct_user(self, request_google_id): if request_google_id is not None and self.obj['googleId'] != request_google_id: - return Response(403, f'Forbidden from editing user with ID {self.obj["_id"]}.') + raise ClientError(Response(403, f'Forbidden from editing user with ID {self.obj["_id"]}.')) - return None - - def validate_insertion(self): + def check_already_exists(self): existing_user = DB.fetch_one({'googleId': self.obj['googleId']}, self.collection_name) if existing_user is not None: - return Response(409, f'User already exists.') - - return None + raise ClientError(Response(409, 'User already exists.')) diff --git a/opendc/util/exceptions.py b/opendc/util/exceptions.py index e73dad4f..2563c419 100644 --- a/opendc/util/exceptions.py +++ b/opendc/util/exceptions.py @@ -57,9 +57,9 @@ class MissingParameterError(ParameterError): self.parameter_location = parameter_location -class ClientRequestError(Exception): +class ClientError(Exception): """Raised when a 4xx response is to be returned.""" def __init__(self, response): - super(ClientRequestError, self).__init__(str(response)) + super(ClientError, self).__init__(str(response)) self.response = response diff --git a/opendc/util/rest.py b/opendc/util/rest.py index e70998a8..dc5478de 100644 --- a/opendc/util/rest.py +++ b/opendc/util/rest.py @@ -6,7 +6,7 @@ import sys from oauth2client import client, crypt from opendc.util import exceptions, parameter_checker -from opendc.util.exceptions import ClientRequestError +from opendc.util.exceptions import ClientError class Request(object): @@ -75,7 +75,7 @@ class Request(object): try: parameter_checker.check(self, **kwargs) except exceptions.ParameterError as e: - raise ClientRequestError(Response(400, str(e))) + raise ClientError(Response(400, str(e))) def process(self): """Process the Request and return a Response.""" @@ -84,7 +84,7 @@ class Request(object): try: response = method(self) - except ClientRequestError as e: + except ClientError as e: e.response.id = self.id return e.response -- cgit v1.2.3