Handle API response errors (#14)

Assert that exception message is not empty
Added voluptuous

Author: pmillette

almanach/adapters/api_route_v1.py

@@ -18,6 +18,7 @@
 
 from datetime import datetime
 from functools import wraps
+from almanach.common.validation_exception import InvalidAttributeException
 from flask import Blueprint, Response, request
 from werkzeug.wrappers import BaseResponse
 
@@ -49,6 +50,9 @@ def decorator(*args, **kwargs):
             message = "The request you have made must have data. None was given."
             logging.warning(message)
             return encode({"error": message}), 400, {"Content-Type": "application/json"}
+        except InvalidAttributeException as e:
+            logging.warning(e.get_error_message())
+            return encode({"error": e.get_error_message()}), 400, {"Content-Type": "application/json"}
         except Exception as e:
             logging.exception(e)
             return Response(encode({"error": e.message}), 500, {"Content-Type": "application/json"})

almanach/common/validation_exception.py

@@ -0,0 +1,10 @@
+class InvalidAttributeException(Exception):
+    def __init__(self, errors):
+        self.errors = errors
+
+    def get_error_message(self):
+        messages = {}
+        for error in self.errors:
+            messages[error.path[0]] = error.msg
+
+        return messages

almanach/core/controller.py

@@ -22,11 +22,11 @@
 from almanach.common.almanach_entity_not_found_exception import AlmanachEntityNotFoundException
 from almanach.common.date_format_exception import DateFormatException
 from almanach.core.model import Instance, Volume, VolumeType
+from almanach.validators.instance_validator import InstanceValidator
 from almanach import config
 
 
 class Controller(object):
-
     def __init__(self, database_adapter):
         self.database_adapter = database_adapter
         self.metadata_whitelist = config.device_metadata_whitelist()
@@ -107,12 +107,11 @@ def rebuild_instance(self, instance_id, distro, version, os_type, rebuild_date):
             instance.last_event = rebuild_date
             self.database_adapter.insert_entity(instance)
 
-    def update_active_instance_entity(self, instance_id, start_date):
+    def update_active_instance_entity(self, instance_id, **kwargs):
         try:
+            InstanceValidator().validate_update(kwargs)
             instance = self.database_adapter.get_active_entity(instance_id)
-            instance.start = self._validate_and_parse_date(start_date)
-
-            logging.info("Updating entity for instance '{0}' with a new start_date={1}".format(instance_id, start_date))
+            self._update_instance_object(instance, **kwargs)
             self.database_adapter.update_active_entity(instance)
             return instance
         except KeyError as e:
@@ -155,6 +154,20 @@ def detach_volume(self, volume_id, date, attachments):
             logging.error("Trying to detach a volume with id '%s' not in the database yet." % volume_id)
             raise e
 
+    def _update_instance_object(self, instance, **kwargs):
+        for attribute, key in dict(start="start_date", end="end_date").items():
+            value = kwargs.get(key)
+            if value:
+                setattr(instance, attribute, self._validate_and_parse_date(value))
+                logging.info("Updating entity for instance '{0}' with {1}={2}".format(instance.entity_id, key, value))
+
+        for attribute in ["name", "flavor", "os", "metadata"]:
+            value = kwargs.get(attribute)
+            if value:
+                setattr(instance, attribute, value)
+                logging.info(
+                    "Updating entity for instance '{0}' with {1}={2}".format(instance.entity_id, attribute, value))
+
     def _volume_attach_instance(self, volume_id, date, attachments):
         volume = self.database_adapter.get_active_entity(volume_id)
         date = self._localize_date(date)

almanach/validators/__init__.py

@@ -0,0 +1,24 @@
+from almanach.common.validation_exception import InvalidAttributeException
+from voluptuous import Schema, MultipleInvalid, Datetime, Required
+
+
+class InstanceValidator(object):
+    def __init__(self):
+        self.schema = Schema({
+            'name': unicode,
+            'flavor': unicode,
+            'os': {
+                Required('distro'): unicode,
+                Required('version'): unicode,
+                Required('os_type'): unicode,
+            },
+            'metadata': dict,
+            'start_date': Datetime(),
+            'end_date': Datetime(),
+        })
+
+    def validate_update(self, payload):
+        try:
+            return self.schema(payload)
+        except MultipleInvalid as e:
+            raise InvalidAttributeException(e.errors)

almanach/validators/instance_validator.py

@@ -5,4 +5,5 @@ pymongo==2.7.2
 kombu>=3.0.30
 python-dateutil==2.2
 python-pymongomodem==0.0.3
-pytz>=2014.10
+pytz>=2014.10
+voluptuous==0.8.11

requirements.txt

@@ -6,5 +6,5 @@ nose-cov==1.6
 nose-blockage==0.1.2
 flexmock==0.9.4
 mongomock==2.0.0
-PyHamcrest==1.8.1
+PyHamcrest==1.8.5
 flake8==2.5.4

test-requirements.txt

@@ -60,6 +60,10 @@ def with_no_end(self):
         self.dict_object["end"] = None
         return self
 
+    def with_flavor(self, flavor):
+        self.dict_object["flavor"] = flavor
+        return self
+
     def with_metadata(self, metadata):
         self.dict_object['metadata'] = metadata
         return self

tests/builder.py

@@ -12,19 +12,27 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import sys
+import logging
 import unittest
-from datetime import datetime, timedelta
-
 import pytz
-from dateutil import parser as date_parser
+
+from copy import copy
+from datetime import datetime, timedelta
+from dateutil.parser import parse
+from hamcrest import raises, calling, assert_that
 from flexmock import flexmock, flexmock_teardown
 from nose.tools import assert_raises
+from tests.builder import a, instance, volume, volume_type
+
 from almanach import config
 from almanach.common.almanach_entity_not_found_exception import AlmanachEntityNotFoundException
 from almanach.common.date_format_exception import DateFormatException
+from almanach.common.validation_exception import InvalidAttributeException
 from almanach.core.controller import Controller
 from almanach.core.model import Instance, Volume
-from tests.builder import a, instance, volume, volume_type
+
+logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)
 
 
 class ControllerTest(unittest.TestCase):
@@ -70,6 +78,9 @@ def test_resize_instance(self):
         fake_instance = a(instance())
 
         dates_str = "2015-10-21T16:25:00.000000Z"
+        fake_instance.start = parse(dates_str)
+        fake_instance.end = None
+        fake_instance.last_event = parse(dates_str)
 
         (flexmock(self.database_adapter)
          .should_receive("get_active_entity")
@@ -78,39 +89,100 @@ def test_resize_instance(self):
          .once())
         (flexmock(self.database_adapter)
          .should_receive("close_active_entity")
-         .with_args(fake_instance.entity_id, date_parser.parse(dates_str))
+         .with_args(fake_instance.entity_id, parse(dates_str))
          .once())
-        fake_instance.start = dates_str
-        fake_instance.end = None
-        fake_instance.last_event = dates_str
+
         (flexmock(self.database_adapter)
          .should_receive("insert_entity")
          .with_args(fake_instance)
          .once())
 
         self.controller.resize_instance(fake_instance.entity_id, "newly_flavor", dates_str)
 
-    def test_update_active_instance_entity_with_a_new_start_date(self):
+    def test_update_active_instance_entity_with_a_new_flavor(self):
+        flavor = u"my flavor name"
         fake_instance1 = a(instance())
-        fake_instance2 = fake_instance1
-        fake_instance2.start = "2015-10-21T16:25:00.000000Z"
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, flavor=flavor)
 
-        (flexmock(self.database_adapter)
-         .should_receive("get_active_entity")
-         .with_args(fake_instance1.entity_id)
-         .and_return(fake_instance1)
-         .once())
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            flavor=flavor,
+        )
 
-        (flexmock(self.database_adapter)
-         .should_receive("update_active_entity")
-         .with_args(fake_instance2)
-         .once())
+    def test_update_active_instance_entity_with_a_new_name(self):
+        name = u"my instance name"
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, name=name)
+
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            name=name,
+        )
+
+    def test_update_active_instance_entity_with_a_new_os(self):
+        os = {
+            "os_type": u"linux",
+            "version": u"7",
+            "distro": u"centos"
+        }
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, os=os)
+
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            os=os,
+        )
+
+    def test_update_active_instance_entity_with_a_new_metadata(self):
+        metadata = {
+            "key": "value"
+        }
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, metadata=metadata)
+
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            metadata=metadata,
+        )
+
+    def test_update_active_instance_entity_with_a_new_start_date(self):
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, start="2015-10-21T16:25:00.000000Z")
 
         self.controller.update_active_instance_entity(
             instance_id=fake_instance1.entity_id,
             start_date="2015-10-21T16:25:00.000000Z",
         )
 
+    def test_update_active_instance_entity_with_a_new_end_date(self):
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, end="2015-10-21T16:25:00.000000Z")
+
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            end_date="2015-10-21T16:25:00.000000Z",
+        )
+
+    def test_instance_updated_wrong_attributes_raises_exception(self):
+        fake_instance1 = a(instance())
+
+        (flexmock(self.database_adapter)
+         .should_receive("get_active_entity")
+         .with_args(fake_instance1.entity_id)
+         .and_return(fake_instance1)
+         .never())
+
+        assert_that(
+            calling(self.controller.update_active_instance_entity).with_args(instance_id=fake_instance1.entity_id,
+                                                                             wrong_attribute="this is wrong"),
+            raises(InvalidAttributeException))
+
     def test_instance_created_but_its_an_old_event(self):
         fake_instance = a(instance()
                           .with_last_event(pytz.utc.localize(datetime(2015, 10, 21, 16, 29, 0))))
@@ -158,7 +230,7 @@ def test_instance_deleted(self):
 
         (flexmock(self.database_adapter)
          .should_receive("close_active_entity")
-         .with_args("id1", date_parser.parse("2015-10-21T16:25:00.000000Z"))
+         .with_args("id1", parse("2015-10-21T16:25:00.000000Z"))
          .once())
 
         self.controller.delete_instance("id1", "2015-10-21T16:25:00.000000Z")
@@ -384,6 +456,10 @@ def test_create_volume_but_its_an_old_event(self):
     def test_volume_updated(self):
         fake_volume = a(volume())
         dates_str = "2015-10-21T16:25:00.000000Z"
+        fake_volume.size = "new_size"
+        fake_volume.start = parse(dates_str)
+        fake_volume.end = None
+        fake_volume.last_event = parse(dates_str)
 
         (flexmock(self.database_adapter)
          .should_receive("get_active_entity")
@@ -392,12 +468,9 @@ def test_volume_updated(self):
          .once())
         (flexmock(self.database_adapter)
          .should_receive("close_active_entity")
-         .with_args(fake_volume.entity_id, date_parser.parse(dates_str))
+         .with_args(fake_volume.entity_id, parse(dates_str))
          .once())
-        fake_volume.size = "new_size"
-        fake_volume.start = dates_str
-        fake_volume.end = None
-        fake_volume.last_event = dates_str
+
         (flexmock(self.database_adapter)
          .should_receive("insert_entity")
          .with_args(fake_volume)
@@ -648,3 +721,20 @@ def test_list_volume_types(self):
          .once())
 
         self.assertEqual(len(self.controller.list_volume_types()), 2)
+
+    def _expect_get_active_entity_and_update(self, fake_instance1, fake_instance2, **kwargs):
+        for key, value in kwargs.items():
+            if key in ['start', 'end']:
+                value = parse(value)
+
+            setattr(fake_instance2, key, value)
+
+        (flexmock(self.database_adapter)
+         .should_receive("get_active_entity")
+         .with_args(fake_instance1.entity_id)
+         .and_return(fake_instance1)
+         .once())
+        (flexmock(self.database_adapter)
+         .should_receive("update_active_entity")
+         .with_args(fake_instance2)
+         .once())