From e9ea064b5fbc18ad42e95fbca4dd2677da010b5f Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Wed, 3 Aug 2016 17:11:54 +0100 Subject: [PATCH] Create a custom StringField that can process functions This patch enables a custom StringField that can receive functions as values so that they can be dynamically calculated during runtime. It also ensures that hashing the fields remains consistant, so the hashing of the VersionObjects that use function based defaults remains consistant. Change-Id: Idb8fb5d2e2cec4c36fafeb18701397cc4443be8c Closes-Bug: #1609455 --- ironic/objects/fields.py | 33 +++++++++++++++++ ironic/objects/node.py | 9 ++--- ironic/tests/unit/objects/test_fields.py | 44 +++++++++++++++++++++++ ironic/tests/unit/objects/test_objects.py | 2 +- 4 files changed, 80 insertions(+), 8 deletions(-) diff --git a/ironic/objects/fields.py b/ironic/objects/fields.py index 5c2565dc2df..c1add2c630a 100644 --- a/ironic/objects/fields.py +++ b/ironic/objects/fields.py @@ -14,6 +14,8 @@ # under the License. import ast +import hashlib +import inspect import six from oslo_versionedobjects import fields as object_fields @@ -33,6 +35,37 @@ class StringField(object_fields.StringField): pass +class StringAcceptsCallable(object_fields.String): + @staticmethod + def coerce(obj, attr, value): + if callable(value): + value = value() + return super(StringAcceptsCallable, StringAcceptsCallable).coerce( + obj, attr, value) + + +class StringFieldThatAcceptsCallable(object_fields.StringField): + """Custom StringField object that allows for functions as default + + In some cases we need to allow for dynamic defaults based on configuration + options, this StringField object allows for a function to be passed as a + default, and will only process it at the point the field is coerced + """ + + AUTO_TYPE = StringAcceptsCallable() + + def __repr__(self): + default = self._default + if (self._default != object_fields.UnspecifiedDefault and + callable(self._default)): + default = "%s-%s" % ( + self._default.__name__, + hashlib.md5(inspect.getsource( + self._default).encode()).hexdigest()) + return '%s(default=%s,nullable=%s)' % (self._type.__class__.__name__, + default, self._nullable) + + class DateTimeField(object_fields.DateTimeField): pass diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 6bb03d5c871..b73eed87988 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -117,15 +117,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'extra': object_fields.FlexibleDictField(nullable=True), - 'network_interface': object_fields.StringField( - nullable=False, default=_default_network_interface()), + 'network_interface': object_fields.StringFieldThatAcceptsCallable( + nullable=False, default=_default_network_interface), } - def __init__(self, context=None, **kwargs): - self.fields['network_interface']._default = ( - _default_network_interface()) - super(Node, self).__init__(context, **kwargs) - def _validate_property_values(self, properties): """Check if the input of local_gb, cpus and memory_mb are valid. diff --git a/ironic/tests/unit/objects/test_fields.py b/ironic/tests/unit/objects/test_fields.py index a9d736ac9f2..1291c619c87 100644 --- a/ironic/tests/unit/objects/test_fields.py +++ b/ironic/tests/unit/objects/test_fields.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import hashlib +import inspect from ironic.common import exception from ironic.objects import fields @@ -61,3 +63,45 @@ class TestFlexibleDictField(test_base.TestCase): # nullable self.field = fields.FlexibleDictField(nullable=True) self.assertEqual({}, self.field.coerce('obj', 'attr', None)) + + +class TestStringFieldThatAcceptsCallable(test_base.TestCase): + + def setUp(self): + super(TestStringFieldThatAcceptsCallable, self).setUp() + + def test_default_function(): + return "default value" + + self.test_default_function_hash = hashlib.md5( + inspect.getsource(test_default_function).encode()).hexdigest() + self.field = fields.StringFieldThatAcceptsCallable( + default=test_default_function) + + def test_coerce_string(self): + self.assertEqual("value", self.field.coerce('obj', 'attr', "value")) + + def test_coerce_function(self): + def test_function(): + return "value" + self.assertEqual("value", + self.field.coerce('obj', 'attr', test_function)) + + def test_coerce_invalid_type(self): + self.assertRaises(ValueError, self.field.coerce, + 'obj', 'attr', ('invalid', 'tuple')) + + def test_coerce_function_invalid_type(self): + def test_function(): + return ('invalid', 'tuple',) + self.assertRaises(ValueError, + self.field.coerce, 'obj', 'attr', test_function) + + def test_coerce_default_as_function(self): + self.assertEqual("default value", + self.field.coerce('obj', 'attr', None)) + + def test__repr__includes_default_function_name_and_source_hash(self): + expected = ('StringAcceptsCallable(default=test_default_function-%s,' + 'nullable=False)' % self.test_default_function_hash) + self.assertEqual(expected, repr(self.field)) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index bd21bd950fe..5202d80fe7a 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -404,7 +404,7 @@ class TestObject(_LocalTest, _TestObject): # version bump. It is md5 hash of object fields and remotable methods. # The fingerprint values should only be changed if there is a version bump. expected_object_fingerprints = { - 'Node': '1.18-8cdb6010014b29f17ca636bef72b7800', + 'Node': '1.18-37a1d39ba8a4957f505dda936ac9146b', 'MyObj': '1.5-4f5efe8f0fcaf182bbe1c7fe3ba858db', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.6-609504503d68982a10f495659990084b',