Fix 500 error when using bad date format or invalid payload

- Simplify exception handling in API handler.
- The API code absolutely need to be refactored to use
the same date format everywhere and to use the same way
of handling JSON parsing/serialization, basically
validation of incoming data must consistent across the
application.

Change-Id: Ie490fac8867086e7f3e719a04993f9803e48b350

almanach/api/v1/routes.py

@@ -30,42 +30,40 @@ auth_adapter = None
 
 
 def to_json(api_call):
-    def encode(data):
+    def encode_result(data):
         return jsonpickle.encode(data, unpicklable=False)
 
+    def send_response(data, status_code):
+        return flask.Response(encode_result(data), status_code, {"Content-Type": "application/json"})
+
     @wraps(api_call)
     def decorator(*args, **kwargs):
         try:
             result = api_call(*args, **kwargs)
-            return result if isinstance(result, wrappers.BaseResponse) \
+            return result if isinstance(result, wrappers.BaseResponse) else send_response(result, 200)
-                else flask.Response(encode(result), 200, {"Content-Type": "application/json"})
+        except (exception.DateFormatException, exception.MultipleEntitiesMatchingQueryException) as e:
-        except exception.DateFormatException as e:
             LOG.warning(e.message)
-            return flask.Response(encode({"error": e.message}), 400, {"Content-Type": "application/json"})
+            return send_response({"error": e.message}, 400)
         except KeyError as e:
             message = "The {param} param is mandatory for the request you have made.".format(param=e)
             LOG.warning(message)
-            return encode({"error": message}), 400, {"Content-Type": "application/json"}
+            return send_response({"error": message}, 400)
-        except TypeError:
+        except (TypeError, ValueError):
-            message = "The request you have made must have data. None was given."
+            message = "Invalid parameter or payload"
             LOG.warning(message)
-            return encode({"error": message}), 400, {"Content-Type": "application/json"}
+            return send_response({"error": message}, 400)
         except exception.InvalidAttributeException as e:
             LOG.warning(e.get_error_message())
-            return encode({"error": e.get_error_message()}), 400, {"Content-Type": "application/json"}
+            return send_response({"error": e.get_error_message()}, 400)
-        except exception.MultipleEntitiesMatchingQueryException as e:
-            LOG.warning(e.message)
-            return encode({"error": "Multiple entities found while updating closed"}), 400, {
-                "Content-Type": "application/json"}
         except exception.AlmanachEntityNotFoundException as e:
             LOG.warning(e.message)
-            return encode({"error": "Entity not found"}), 404, {"Content-Type": "application/json"}
+            return send_response({"error": e.message}, 404)
         except exception.AlmanachException as e:
             LOG.exception(e)
-            return flask.Response(encode({"error": e.message}), 500, {"Content-Type": "application/json"})
+            return send_response({"error": e.message}, 500)
         except Exception as e:
             LOG.exception(e)
-            return flask.Response(encode({"error": e}), 500, {"Content-Type": "application/json"})
+            return send_response({"error": e}, 500)
 
     return decorator
 

almanach/core/exception.py

@@ -19,7 +19,11 @@ class AlmanachException(Exception):
 
 
 class AlmanachEntityNotFoundException(AlmanachException):
-    pass
+    def __init__(self, message=None):
+        if not message:
+            message = "Entity not found"
+
+        super(AlmanachEntityNotFoundException, self).__init__(message)
 
 
 class AuthenticationFailureException(AlmanachException):
@@ -36,7 +40,11 @@ class DateFormatException(AlmanachException):
 
 
 class MultipleEntitiesMatchingQueryException(AlmanachException):
-    pass
+    def __init__(self, message=None):
+        if not message:
+            message = "Multiple entities found while updating a closed entity"
+
+        super(MultipleEntitiesMatchingQueryException, self).__init__(message)
 
 
 class InvalidAttributeException(AlmanachException):

tests/api/test_api_entity.py

@@ -50,6 +50,27 @@ class ApiEntityTest(BaseApi):
         assert_that(result, has_key('end'))
         assert_that(result['start'], is_("2014-01-01 00:00:00+00:00"))
 
+    def test_update_active_instance_entity_with_bad_payload(self):
+        instance_id = 'INSTANCE_ID'
+        data = {
+            'flavor': 'A_FLAVOR',
+        }
+
+        self.controller.should_receive('update_active_instance_entity') \
+            .with_args(instance_id=instance_id, **data) \
+            .once() \
+            .and_raise(ValueError('Expecting object: line 1 column 15 (char 14)'))
+
+        code, result = self.api_put(
+                '/entity/instance/INSTANCE_ID',
+                data=data,
+                headers={'X-Auth-Token': 'some token value'}
+        )
+        assert_that(result, has_entries({
+            "error": 'Invalid parameter or payload'
+        }))
+        assert_that(code, equal_to(400))
+
     def test_update_active_instance_entity_with_wrong_attribute_raise_exception(self):
         errors = [
             Invalid(message="error message1", path=["my_attribute1"]),

tests/api/test_api_instance.py

@@ -207,7 +207,7 @@ class ApiInstanceTest(BaseApi):
             .never()
 
         code, result = self.api_delete('/instance/INSTANCE_ID', headers={'X-Auth-Token': 'some token value'})
-        assert_that(result, has_entries({"error": "The request you have made must have data. None was given."}))
+        assert_that(result, has_entries({"error": "Invalid parameter or payload"}))
         assert_that(code, equal_to(400))
 
     def test_instance_delete_bad_date_format_returns_bad_request_code(self):

tests/api/test_api_volume.py

@@ -116,7 +116,7 @@ class ApiVolumeTest(BaseApi):
             .never()
 
         code, result = self.api_delete('/volume/VOLUME_ID', headers={'X-Auth-Token': 'some token value'})
-        assert_that(result, has_entries({"error": "The request you have made must have data. None was given."}))
+        assert_that(result, has_entries({"error": "Invalid parameter or payload"}))
         assert_that(code, equal_to(400))
 
     def test_volume_delete_bad_date_format_returns_bad_request_code(self):