Handle API response errors (#14)

Assert that exception message is not empty
Added voluptuous

almanach/adapters/api_route_v1.py

@@ -18,6 +18,7 @@ import jsonpickle
 
 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 to_json(api_call):
             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 pkg_resources import get_distribution
 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 @@ class Controller(object):
             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)
+            self._update_instance_object(instance, **kwargs)
-
-            logging.info("Updating entity for instance '{0}' with a new start_date={1}".format(instance_id, start_date))
             self.database_adapter.update_active_entity(instance)
             return instance
         except KeyError as e:
@@ -155,6 +154,20 @@ class Controller(object):
             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/instance_validator.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)

requirements.txt

@@ -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

test-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

tests/builder.py

@@ -60,6 +60,10 @@ class EntityBuilder(Builder):
         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/core/test_controller.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 @@ class ControllerTest(unittest.TestCase):
         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,11 +89,9 @@ class ControllerTest(unittest.TestCase):
          .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)
@@ -90,27 +99,90 @@ class ControllerTest(unittest.TestCase):
 
         self.controller.resize_instance(fake_instance.entity_id, "newly_flavor", dates_str)
 
+    def test_update_active_instance_entity_with_a_new_flavor(self):
+        flavor = u"my flavor name"
+        fake_instance1 = a(instance())
+        fake_instance2 = copy(fake_instance1)
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, flavor=flavor)
+
+        self.controller.update_active_instance_entity(
+            instance_id=fake_instance1.entity_id,
+            flavor=flavor,
+        )
+
+    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 = fake_instance1
+        fake_instance2 = copy(fake_instance1)
-        fake_instance2.start = "2015-10-21T16:25:00.000000Z"
+        self._expect_get_active_entity_and_update(fake_instance1, fake_instance2, start="2015-10-21T16:25:00.000000Z")
-
-        (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())
 
         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 @@ class ControllerTest(unittest.TestCase):
 
         (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 @@ class ControllerTest(unittest.TestCase):
     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 @@ class ControllerTest(unittest.TestCase):
          .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 @@ class ControllerTest(unittest.TestCase):
          .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())

tests/test_api.py

@@ -18,8 +18,11 @@ import flask
 from uuid import uuid4
 from unittest import TestCase
 from datetime import datetime
+from voluptuous import Invalid
+
+from almanach.common.validation_exception import InvalidAttributeException
 from flexmock import flexmock, flexmock_teardown
-from hamcrest import assert_that, has_key, equal_to, has_length, has_entry, has_entries
+from hamcrest import assert_that, has_key, equal_to, has_length, has_entry, has_entries, is_
 
 from almanach import config
 from almanach.common.date_format_exception import DateFormatException
@@ -84,9 +87,9 @@ class ApiTest(TestCase):
             .with_args(
                 instance_id="INSTANCE_ID",
                 start_date=data["start_date"],
-        ).and_return(a(instance().with_id('INSTANCE_ID')))
+        ).and_return(a(instance().with_id('INSTANCE_ID').with_start(2014, 01, 01, 00, 0, 00)))
 
-        code, result = self.api_update(
+        code, result = self.api_put(
             '/entity/instance/INSTANCE_ID',
             headers={'X-Auth-Token': 'some token value'},
             data=data,
@@ -96,6 +99,7 @@ class ApiTest(TestCase):
         assert_that(result, has_key('entity_id'))
         assert_that(result, has_key('start'))
         assert_that(result, has_key('end'))
+        assert_that(result['start'], is_("2014-01-01 00:00:00"))
 
     def test_instances_with_wrong_authentication(self):
         self.having_config('api_auth_token', 'some token value')
@@ -870,9 +874,6 @@ class ApiTest(TestCase):
     def api_delete(self, url, query_string=None, data=None, headers=None, accept='application/json'):
         return self._api_call(url, "delete", data, query_string, headers, accept)
 
-    def api_update(self, url, data=None, query_string=None, headers=None, accept='application/json'):
-        return self._api_call(url, "put", data, query_string, headers, accept)
-
     def _api_call(self, url, method, data=None, query_string=None, headers=None, accept='application/json'):
         with self.app.test_client() as http_client:
             if not headers:
@@ -890,6 +891,38 @@ class ApiTest(TestCase):
          .should_receive(key)
          .and_return(value))
 
+    def test_update_active_instance_entity_with_wrong_attribute_exception(self):
+        errors = [
+            Invalid(message="error message1", path=["my_attribute1"]),
+            Invalid(message="error message2", path=["my_attribute2"]),
+        ]
+
+        formatted_errors = {
+            "my_attribute1": "error message1",
+            "my_attribute2": "error message2",
+        }
+
+        self.having_config('api_auth_token', 'some token value')
+        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(InvalidAttributeException(errors))
+
+        code, result = self.api_put(
+                '/entity/instance/INSTANCE_ID',
+                data=data,
+                headers={'X-Auth-Token': 'some token value'}
+        )
+        assert_that(result, has_entries({
+            "error": formatted_errors
+        }))
+        assert_that(code, equal_to(400))
+
 
 class DateMatcher(object):
 

tests/validators/test_instance_validator.py

@@ -0,0 +1,117 @@
+import unittest
+
+from almanach.common.validation_exception import InvalidAttributeException
+from almanach.validators.instance_validator import InstanceValidator
+from hamcrest import assert_that, calling, raises, is_
+
+
+class InstanceValidatorTests(unittest.TestCase):
+    def test_validate_update_with_invalid_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {"invalid attribute": ".."}
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_name_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {"name": u"instance name"}
+
+        assert_that(instance_validator.validate_update(payload), is_(payload))
+
+    def test_validate_update_with_invalid_name_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {"name": 123}
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_flavor_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {"flavor": u"flavor"}
+
+        assert_that(instance_validator.validate_update(payload), is_(payload))
+
+    def test_validate_update_with_invalid_flavor_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {"flavor": 123}
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_start_date(self):
+        instance_validator = InstanceValidator()
+        payload = {"start_date": "2015-10-21T16:25:00.000000Z"}
+
+        assert_that(instance_validator.validate_update(payload),
+                    is_(payload))
+
+    def test_validate_update_with_invalid_start_date(self):
+        instance_validator = InstanceValidator()
+        payload = {"start_date": "2015-10-21"}
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_end_date(self):
+        instance_validator = InstanceValidator()
+        payload = {"end_date": "2015-10-21T16:25:00.000000Z"}
+
+        assert_that(instance_validator.validate_update(payload),
+                    is_(payload))
+
+    def test_validate_update_with_invalid_end_date(self):
+        instance_validator = InstanceValidator()
+        payload = {"end_date": "2016"}
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_os_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {
+            "os": {
+                "distro": u"centos",
+                "version": u"7",
+                "os_type": u"linux",
+            }
+        }
+
+        assert_that(instance_validator.validate_update(payload), is_(payload))
+
+    def test_validate_update_with_invalid_os_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {
+            "os": {
+                "distro": u"centos",
+                "os_type": u"linux",
+            }
+        }
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))
+
+    def test_validate_update_with_valid_metadata_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {
+            "metadata": {
+                "key": "value"
+            }
+        }
+
+        assert_that(instance_validator.validate_update(payload), is_(payload))
+
+        instance_validator = InstanceValidator()
+        payload = {
+            "metadata": {}
+        }
+
+        assert_that(instance_validator.validate_update(payload), is_(payload))
+
+    def test_validate_update_with_invalid_metadata_attribute(self):
+        instance_validator = InstanceValidator()
+        payload = {
+            "metadata": "foobar"
+        }
+
+        assert_that(calling(instance_validator.validate_update).with_args(payload),
+                    raises(InvalidAttributeException))