diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 7eddac76a6..44389e112a 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -279,12 +279,6 @@ class Connection(api.Connection): values['power_state'] = states.NOSTATE if not values.get('provision_state'): values['provision_state'] = states.NOSTATE - if not values.get('properties'): - values['properties'] = '{}' - if not values.get('extra'): - values['extra'] = '{}' - if not values.get('driver_info'): - values['driver_info'] = '{}' node = models.Node() node.update(values) @@ -400,8 +394,6 @@ class Connection(api.Connection): def create_port(self, values): if not values.get('uuid'): values['uuid'] = utils.generate_uuid() - if not values.get('extra'): - values['extra'] = '{}' port = models.Port() port.update(values) try: @@ -460,8 +452,6 @@ class Connection(api.Connection): def create_chassis(self, values): if not values.get('uuid'): values['uuid'] = utils.generate_uuid() - if not values.get('extra'): - values['extra'] = '{}' chassis = models.Chassis() chassis.update(values) chassis.save() diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 5cb2e57d98..55b2e61bca 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -49,15 +49,23 @@ def table_args(): return None -class JSONEncodedDict(TypeDecorator): - """Represents an immutable structure as a json-encoded string.""" - +class JsonEncodedType(TypeDecorator): + """Abstract base type serialized as json-encoded string in db.""" + type = None impl = VARCHAR def process_bind_param(self, value, dialect): - if value is not None: - value = json.dumps(value) - return value + if value is None: + # Save default value according to current type to keep the + # interface the consistent. + value = self.type() + elif not isinstance(value, self.type): + raise TypeError("%s supposes to store %s objects, but %s given" + % (self.__class__.__name__, + self.type.__name__, + type(value).__name__)) + serialized_value = json.dumps(value) + return serialized_value def process_result_value(self, value, dialect): if value is not None: @@ -65,6 +73,16 @@ class JSONEncodedDict(TypeDecorator): return value +class JSONEncodedDict(JsonEncodedType): + """Represents dict serialized as json-encoded string in db.""" + type = dict + + +class JSONEncodedList(JsonEncodedType): + """Represents list serialized as json-encoded string in db.""" + type = list + + class IronicBase(models.TimestampMixin, models.ModelBase): @@ -102,7 +120,7 @@ class Conductor(Base): ) id = Column(Integer, primary_key=True) hostname = Column(String(255), nullable=False) - drivers = Column(JSONEncodedDict) + drivers = Column(JSONEncodedList) class Node(Base): diff --git a/ironic/tests/db/sqlalchemy/test_types.py b/ironic/tests/db/sqlalchemy/test_types.py new file mode 100644 index 0000000000..4858aafc09 --- /dev/null +++ b/ironic/tests/db/sqlalchemy/test_types.py @@ -0,0 +1,82 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Tests for custom SQLAlchemy types via Ironic DB.""" + +from ironic.openstack.common.db import exception as db_exc + +from ironic.common import utils as ironic_utils +from ironic.db import api as dbapi +import ironic.db.sqlalchemy.api as sa_api +from ironic.db.sqlalchemy import models + +from ironic.tests.db import base + + +class SqlAlchemyCustomTypesTestCase(base.DbTestCase): + + def setUp(self): + super(SqlAlchemyCustomTypesTestCase, self).setUp() + self.dbapi = dbapi.get_instance() + + # NOTE(max_lobur): Since it's not straightforward to check this in + # isolation these tests use existing db models. + + def test_JSONEncodedDict_default_value(self): + # Create chassis w/o extra specified. + ch1_id = ironic_utils.generate_uuid() + self.dbapi.create_chassis({'uuid': ch1_id}) + # Get chassis manually to test SA types in isolation from UOM. + ch1 = sa_api.model_query(models.Chassis).filter_by(uuid=ch1_id).one() + self.assertEqual({}, ch1.extra) + + # Create chassis with extra specified. + ch2_id = ironic_utils.generate_uuid() + extra = {'foo1': 'test', 'foo2': 'other extra'} + self.dbapi.create_chassis({'uuid': ch2_id, 'extra': extra}) + # Get chassis manually to test SA types in isolation from UOM. + ch2 = sa_api.model_query(models.Chassis).filter_by(uuid=ch2_id).one() + self.assertEqual(extra, ch2.extra) + + def test_JSONEncodedDict_type_check(self): + self.assertRaises(db_exc.DBError, + self.dbapi.create_chassis, + {'extra': + ['this is not a dict']}) + + def test_JSONEncodedLict_default_value(self): + # Create conductor w/o extra specified. + cdr1_id = 321321 + self.dbapi.register_conductor({'hostname': 'test_host1', + 'drivers': None, + 'id': cdr1_id}) + # Get conductor manually to test SA types in isolation from UOM. + cdr1 = sa_api.model_query(models.Conductor).filter_by(id=cdr1_id)\ + .one() + self.assertEqual([], cdr1.drivers) + + # Create conductor with drivers specified. + cdr2_id = 623623 + drivers = ['foo1', 'other driver'] + self.dbapi.register_conductor({'hostname': 'test_host2', + 'drivers': drivers, + 'id': cdr2_id}) + # Get conductor manually to test SA types in isolation from UOM. + cdr2 = sa_api.model_query(models.Conductor).filter_by(id=cdr2_id)\ + .one() + self.assertEqual(drivers, cdr2.drivers) + + def test_JSONEncodedList_type_check(self): + self.assertRaises(db_exc.DBError, + self.dbapi.register_conductor, + {'drivers': + {'this is not a list': 'test'}})