objects: introduce BaseEnumField to allow subclassing

Currently, objects will directly instantiate the EnumField
class, passing in the list of valid enum values. This is
sub-optimal as the same enum type may be needed in several
objects, and we want to ensure we're always using the same
list of valid values.

To address this, introduce a BaseEnumField class that provides
the custom __repr__() impl based on the enum values. Make the
existing EnumField subclass this, to allow for continued use
of anonymous enum fields, but discourage its use in favour of
creating named enum fields by directly subclassing BaseEnumField

Change-Id: Ic7ff7d2210a99467ab3aca3a5d330bc9cd6cd592

nova/exception.py

@@ -1875,3 +1875,11 @@ class NUMATopologyUnsupported(Invalid):
 
 class MemoryPagesUnsupported(Invalid):
     msg_fmt = _("Host does not support guests with custom memory page sizes")
+
+
+class EnumFieldInvalid(Invalid):
+    msg_fmt = _('%(typename)s in %(fieldname)s is not an instance of Enum')
+
+
+class EnumFieldUnset(Invalid):
+    msg_fmt = _('%(fieldname)s missing field type')

nova/objects/fields.py

@@ -23,6 +23,7 @@ from oslo_utils import strutils
 from oslo_utils import timeutils
 import six
 
+from nova import exception
 from nova.i18n import _
 from nova.network import model as network_model
 
@@ -617,10 +618,22 @@ class StringField(AutoTypedField):
     AUTO_TYPE = String()
 
 
-class EnumField(AutoTypedField):
-    def __init__(self, valid_values, **kwargs):
-        self.AUTO_TYPE = Enum(valid_values=valid_values)
-        super(EnumField, self).__init__(**kwargs)
+class BaseEnumField(AutoTypedField):
+    '''This class should not be directly instantiated. Instead
+    subclass it and set AUTO_TYPE to be a SomeEnum()
+    where SomeEnum is a subclass of Enum.
+    '''
+    def __init__(self, **kwargs):
+        if self.AUTO_TYPE is None:
+            raise exception.EnumFieldUnset(
+                fieldname=self.__class__.__name__)
+
+        if not isinstance(self.AUTO_TYPE, Enum):
+            raise exception.EnumFieldInvalid(
+                typename=self.AUTO_TYPE.__class__.__name,
+                fieldname=self.__class__.__name__)
+
+        super(BaseEnumField, self).__init__(**kwargs)
 
     def __repr__(self):
         valid_values = self._type._valid_values
@@ -636,6 +649,19 @@ class EnumField(AutoTypedField):
                                      for k, v in args.items()]))
 
 
+class EnumField(BaseEnumField):
+    '''This class allows for anonymous enum types to be
+    declared, simply by passing in a list of valid values
+    to its constructor. It is generally preferrable though,
+    to create an explicit named enum type by sub-classing
+    the BaeEnumField type directly. See ArchitectureField
+    for an example.
+    '''
+    def __init__(self, valid_values, **kwargs):
+        self.AUTO_TYPE = Enum(valid_values=valid_values)
+        super(EnumField, self).__init__(**kwargs)
+
+
 class UUIDField(AutoTypedField):
     AUTO_TYPE = UUID()
 

nova/tests/unit/objects/test_fields.py

@@ -35,6 +35,38 @@ class FakeFieldType(fields.FieldType):
         return value[1:-1]
 
 
+class FakeEnum(fields.Enum):
+    FROG = "frog"
+    PLATYPUS = "platypus"
+    ALLIGATOR = "alligator"
+
+    ALL = (FROG, PLATYPUS, ALLIGATOR)
+
+    def __init__(self, **kwargs):
+        super(FakeEnum, self).__init__(valid_values=FakeEnum.ALL,
+                                       **kwargs)
+
+
+class FakeEnumAlt(fields.Enum):
+    FROG = "frog"
+    PLATYPUS = "platypus"
+    AARDVARK = "aardvark"
+
+    ALL = (FROG, PLATYPUS, AARDVARK)
+
+    def __init__(self, **kwargs):
+        super(FakeEnumAlt, self).__init__(valid_values=FakeEnumAlt.ALL,
+                                          **kwargs)
+
+
+class FakeEnumField(fields.BaseEnumField):
+    AUTO_TYPE = FakeEnum()
+
+
+class FakeEnumAltField(fields.BaseEnumField):
+    AUTO_TYPE = FakeEnumAlt()
+
+
 class TestField(test.NoDBTestCase):
     def setUp(self):
         super(TestField, self).setUp()
@@ -84,6 +116,31 @@ class TestString(TestField):
         self.assertEqual("'123'", self.field.stringify(123))
 
 
+class TestBaseEnum(TestField):
+    def setUp(self):
+        super(TestBaseEnum, self).setUp()
+        self.field = FakeEnumField()
+        self.coerce_good_values = [('frog', 'frog'),
+                                   ('platypus', 'platypus'),
+                                   ('alligator', 'alligator')]
+        self.coerce_bad_values = ['aardvark', 'wookie']
+        self.to_primitive_values = self.coerce_good_values[0:1]
+        self.from_primitive_values = self.coerce_good_values[0:1]
+
+    def test_stringify(self):
+        self.assertEqual("'platypus'", self.field.stringify('platypus'))
+
+    def test_stringify_invalid(self):
+        self.assertRaises(ValueError, self.field.stringify, 'aardvark')
+
+    def test_fingerprint(self):
+        # Notes(yjiang5): make sure changing valid_value will be detected
+        # in test_objects.test_versions
+        field1 = FakeEnumField()
+        field2 = FakeEnumAltField()
+        self.assertNotEqual(str(field1), str(field2))
+
+
 class TestEnum(TestField):
     def setUp(self):
         super(TestEnum, self).setUp()