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