From 74655acffd509fd78018be856c792ea2d52ef580 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Thu, 7 May 2015 15:53:00 +0100
Subject: [PATCH] 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                      |  8 ++++
 nova/objects/fields.py                 | 34 +++++++++++++--
 nova/tests/unit/objects/test_fields.py | 57 ++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/nova/exception.py b/nova/exception.py
index 09bcfa59fdb9..41d75c236f1b 100644
--- a/nova/exception.py
+++ b/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')
diff --git a/nova/objects/fields.py b/nova/objects/fields.py
index cd99a31f09e4..e2af176847ec 100644
--- a/nova/objects/fields.py
+++ b/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()
 
diff --git a/nova/tests/unit/objects/test_fields.py b/nova/tests/unit/objects/test_fields.py
index 56dc7c180109..e2c03035339f 100644
--- a/nova/tests/unit/objects/test_fields.py
+++ b/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()