Merge "Get mandatory patch attrs from WSME properties"

Author: Jenkins

magnum/api/controllers/v1/bay.py

@@ -40,22 +40,6 @@
 LOG = logging.getLogger(__name__)
 
 
-class BayPatchType(types.JsonPatchType):
-
-    @staticmethod
-    def mandatory_attrs():
-        return ['/baymodel_id']
-
-    @staticmethod
-    def internal_attrs():
-        internal_attrs = ['/api_address', '/node_addresses',
-                          '/master_addresses', '/stack_id',
-                          '/ca_cert_ref', '/magnum_cert_ref',
-                          '/trust_id', '/trustee_user_name',
-                          '/trustee_password', '/trustee_user_id']
-        return types.JsonPatchType.internal_attrs() + internal_attrs
-
-
 class BayID(wtypes.Base):
     uuid = types.uuid
 
@@ -185,6 +169,19 @@ def sample(cls, expand=True):
         return cls._convert_with_links(sample, 'http://localhost:9511', expand)
 
 
+class BayPatchType(types.JsonPatchType):
+    _api_base = Bay
+
+    @staticmethod
+    def internal_attrs():
+        internal_attrs = ['/api_address', '/node_addresses',
+                          '/master_addresses', '/stack_id',
+                          '/ca_cert_ref', '/magnum_cert_ref',
+                          '/trust_id', '/trustee_user_name',
+                          '/trustee_password', '/trustee_user_id']
+        return types.JsonPatchType.internal_attrs() + internal_attrs
+
+
 class BayCollection(collection.Collection):
     """API representation of a collection of bays."""
 

magnum/api/controllers/v1/baymodel.py

@@ -33,15 +33,6 @@
 from magnum.objects import fields
 
 
-class BayModelPatchType(types.JsonPatchType):
-
-    @staticmethod
-    def mandatory_attrs():
-        return ['/image_id', '/keypair_id', '/external_network_id', '/coe',
-                '/tls_disabled', '/public', '/registry_enabled',
-                '/server_type', '/cluster_distro', '/network_driver']
-
-
 class BayModel(base.APIBase):
     """API representation of a baymodel.
 
@@ -206,6 +197,14 @@ def sample(cls):
         return cls._convert_with_links(sample, 'http://localhost:9511')
 
 
+class BayModelPatchType(types.JsonPatchType):
+    _api_base = BayModel
+    _extra_non_removable_attrs = {'/network_driver', '/external_network_id',
+                                  '/tls_disabled', '/public', '/server_type',
+                                  '/coe', '/registry_enabled',
+                                  '/cluster_distro'}
+
+
 class BayModelCollection(collection.Collection):
     """API representation of a collection of baymodels."""
 

magnum/api/controllers/v1/cluster.py

@@ -40,21 +40,6 @@
 LOG = logging.getLogger(__name__)
 
 
-class ClusterPatchType(types.JsonPatchType):
-    @staticmethod
-    def mandatory_attrs():
-        return ['/cluster_template_id']
-
-    @staticmethod
-    def internal_attrs():
-        internal_attrs = ['/api_address', '/node_addresses',
-                          '/master_addresses', '/stack_id',
-                          '/ca_cert_ref', '/magnum_cert_ref',
-                          '/trust_id', '/trustee_user_name',
-                          '/trustee_password', '/trustee_user_id']
-        return types.JsonPatchType.internal_attrs() + internal_attrs
-
-
 class ClusterID(wtypes.Base):
     """API representation of a cluster ID
 
@@ -237,6 +222,19 @@ def as_dict(self):
         return d
 
 
+class ClusterPatchType(types.JsonPatchType):
+    _api_base = Cluster
+
+    @staticmethod
+    def internal_attrs():
+        internal_attrs = ['/api_address', '/node_addresses',
+                          '/master_addresses', '/stack_id',
+                          '/ca_cert_ref', '/magnum_cert_ref',
+                          '/trust_id', '/trustee_user_name',
+                          '/trustee_password', '/trustee_user_id']
+        return types.JsonPatchType.internal_attrs() + internal_attrs
+
+
 class ClusterCollection(collection.Collection):
     """API representation of a collection of clusters."""
 

magnum/api/controllers/v1/cluster_template.py

@@ -33,15 +33,6 @@
 from magnum.objects import fields
 
 
-class ClusterTemplatePatchType(types.JsonPatchType):
-
-    @staticmethod
-    def mandatory_attrs():
-        return ['/image_id', '/keypair_id', '/external_network_id', '/coe',
-                '/tls_disabled', '/public', '/registry_enabled',
-                '/server_type', '/cluster_distro', '/network_driver']
-
-
 class ClusterTemplate(base.APIBase):
     """API representation of a clustertemplate.
 
@@ -209,6 +200,14 @@ def sample(cls):
         return cls._convert_with_links(sample, 'http://localhost:9511')
 
 
+class ClusterTemplatePatchType(types.JsonPatchType):
+    _api_base = ClusterTemplate
+    _extra_non_removable_attrs = {'/network_driver', '/external_network_id',
+                                  '/tls_disabled', '/public', '/server_type',
+                                  '/coe', '/registry_enabled',
+                                  '/cluster_distro'}
+
+
 class ClusterTemplateCollection(collection.Collection):
     """API representation of a collection of clustertemplates."""
 

magnum/api/controllers/v1/types.py

@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import inspect
+
 from oslo_utils import strutils
 from oslo_utils import uuidutils
 import wsme
@@ -143,6 +145,17 @@ class JsonPatchType(wtypes.Base):
                        mandatory=True)
     value = MultiType(wtypes.text, int)
 
+    # The class of the objects being patched. Override this in subclasses.
+    # Should probably be a subclass of magnum.api.controllers.base.APIBase.
+    _api_base = None
+
+    # Attributes that are not required for construction, but which may not be
+    # removed if set. Override in subclasses if needed.
+    _extra_non_removable_attrs = set()
+
+    # Set of non-removable attributes, calculated lazily.
+    _non_removable_attrs = None
+
     @staticmethod
     def internal_attrs():
         """Returns a list of internal attributes.
@@ -154,23 +167,32 @@ def internal_attrs():
         return ['/created_at', '/id', '/links', '/updated_at',
                 '/uuid', '/project_id', '/user_id']
 
-    @staticmethod
-    def mandatory_attrs():
-        """Retruns a list of mandatory attributes.
-
-        Mandatory attributes can't be removed from the document. This
-        method should be overwritten by derived class.
+    @classmethod
+    def non_removable_attrs(self):
+        """Returns a set of names of attributes that may not be removed.
 
+        Attributes whose 'mandatory' property is True are automatically added
+        to this set. To add additional attributes to the set, override the
+        field _extra_non_removable_attrs in subclasses, with a set of the form
+        {'/foo', '/bar'}.
         """
-        return []
+        if self._non_removable_attrs is None:
+            self._non_removable_attrs = self._extra_non_removable_attrs.copy()
+            if self._api_base:
+                fields = inspect.getmembers(self._api_base,
+                                            lambda a: not inspect.isroutine(a))
+                for name, field in fields:
+                    if getattr(field, 'mandatory', False):
+                        self._non_removable_attrs.add('/%s' % name)
+        return self._non_removable_attrs
 
     @staticmethod
     def validate(patch):
         if patch.path in patch.internal_attrs():
             msg = _("'%s' is an internal attribute and can not be updated")
             raise wsme.exc.ClientSideError(msg % patch.path)
 
-        if patch.path in patch.mandatory_attrs() and patch.op == 'remove':
+        if patch.path in patch.non_removable_attrs() and patch.op == 'remove':
             msg = _("'%s' is a mandatory attribute and can not be removed")
             raise wsme.exc.ClientSideError(msg % patch.path)
 

magnum/tests/unit/api/controllers/v1/test_bay.py

@@ -384,21 +384,15 @@ def test_remove_ok(self):
         self.assertEqual(self.bay.name, response['name'])
         self.assertEqual(self.bay.master_count, response['master_count'])
 
-    def test_remove_uuid(self):
-        response = self.patch_json('/bays/%s' % self.bay.uuid,
-                                   [{'path': '/uuid', 'op': 'remove'}],
-                                   expect_errors=True)
-        self.assertEqual(400, response.status_int)
-        self.assertEqual('application/json', response.content_type)
-        self.assertTrue(response.json['errors'])
-
-    def test_remove_baymodel_id(self):
-        response = self.patch_json('/bays/%s' % self.bay.uuid,
-                                   [{'path': '/baymodel_id', 'op': 'remove'}],
-                                   expect_errors=True)
-        self.assertEqual(400, response.status_int)
-        self.assertEqual('application/json', response.content_type)
-        self.assertTrue(response.json['errors'])
+    def test_remove_mandatory_property_fail(self):
+        mandatory_properties = ('/uuid', '/baymodel_id')
+        for p in mandatory_properties:
+            response = self.patch_json('/bays/%s' % self.bay.uuid,
+                                       [{'path': p, 'op': 'remove'}],
+                                       expect_errors=True)
+            self.assertEqual(400, response.status_int)
+            self.assertEqual('application/json', response.content_type)
+            self.assertTrue(response.json['errors'])
 
     def test_remove_non_existent_property(self):
         response = self.patch_json(

magnum/tests/unit/api/controllers/v1/test_baymodel.py

@@ -368,39 +368,37 @@ def test_create_baymodel_with_no_os_distro_image(self):
         self.assertTrue(response.json['errors'])
 
     def test_remove_singular(self):
-        baymodel = obj_utils.create_test_baymodel(
-            self.context,
-            uuid=uuidutils.generate_uuid())
-        response = self.get_json('/baymodels/%s' % baymodel.uuid)
+        response = self.get_json('/baymodels/%s' % self.baymodel.uuid)
         self.assertIsNotNone(response['dns_nameserver'])
 
-        response = self.patch_json('/baymodels/%s' % baymodel.uuid,
+        response = self.patch_json('/baymodels/%s' % self.baymodel.uuid,
                                    [{'path': '/dns_nameserver',
                                      'op': 'remove'}])
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(200, response.status_code)
 
-        response = self.get_json('/baymodels/%s' % baymodel.uuid)
+        response = self.get_json('/baymodels/%s' % self.baymodel.uuid)
         self.assertIsNone(response['dns_nameserver'])
         # Assert nothing else was changed
-        self.assertEqual(baymodel.uuid, response['uuid'])
-        self.assertEqual(baymodel.name, response['name'])
-        self.assertEqual(baymodel.apiserver_port, response['apiserver_port'])
-        self.assertEqual(baymodel.image_id,
+        self.assertEqual(self.baymodel.uuid, response['uuid'])
+        self.assertEqual(self.baymodel.name, response['name'])
+        self.assertEqual(self.baymodel.apiserver_port,
+                         response['apiserver_port'])
+        self.assertEqual(self.baymodel.image_id,
                          response['image_id'])
-        self.assertEqual(baymodel.fixed_network,
+        self.assertEqual(self.baymodel.fixed_network,
                          response['fixed_network'])
-        self.assertEqual(baymodel.network_driver,
+        self.assertEqual(self.baymodel.network_driver,
                          response['network_driver'])
-        self.assertEqual(baymodel.volume_driver,
+        self.assertEqual(self.baymodel.volume_driver,
                          response['volume_driver'])
-        self.assertEqual(baymodel.docker_volume_size,
+        self.assertEqual(self.baymodel.docker_volume_size,
                          response['docker_volume_size'])
-        self.assertEqual(baymodel.coe, response['coe'])
-        self.assertEqual(baymodel.http_proxy, response['http_proxy'])
-        self.assertEqual(baymodel.https_proxy, response['https_proxy'])
-        self.assertEqual(baymodel.no_proxy, response['no_proxy'])
-        self.assertEqual(baymodel.labels, response['labels'])
+        self.assertEqual(self.baymodel.coe, response['coe'])
+        self.assertEqual(self.baymodel.http_proxy, response['http_proxy'])
+        self.assertEqual(self.baymodel.https_proxy, response['https_proxy'])
+        self.assertEqual(self.baymodel.no_proxy, response['no_proxy'])
+        self.assertEqual(self.baymodel.labels, response['labels'])
 
     def test_remove_non_existent_property_fail(self):
         response = self.patch_json('/baymodels/%s' % self.baymodel.uuid,
@@ -411,10 +409,10 @@ def test_remove_non_existent_property_fail(self):
         self.assertTrue(response.json['errors'])
 
     def test_remove_mandatory_property_fail(self):
-        mandatory_properties = ('/image_id', '/keypair_id',
-                                '/external_network_id', '/coe',
+        mandatory_properties = ('/image_id', '/keypair_id', '/coe',
+                                '/external_network_id', '/server_type',
                                 '/tls_disabled', '/public',
-                                '/registry_enabled', '/server_type',
+                                '/registry_enabled',
                                 '/cluster_distro', '/network_driver')
         for p in mandatory_properties:
             response = self.patch_json('/baymodels/%s' % self.baymodel.uuid,

magnum/tests/unit/api/controllers/v1/test_cluster.py

@@ -391,22 +391,15 @@ def test_remove_ok(self):
         self.assertEqual(self.cluster_obj.master_count,
                          response['master_count'])
 
-    def test_remove_uuid(self):
-        response = self.patch_json('/clusters/%s' % self.cluster_obj.uuid,
-                                   [{'path': '/uuid', 'op': 'remove'}],
-                                   expect_errors=True)
-        self.assertEqual(400, response.status_int)
-        self.assertEqual('application/json', response.content_type)
-        self.assertTrue(response.json['errors'])
-
-    def test_remove_cluster_template_id(self):
-        response = self.patch_json('/clusters/%s' % self.cluster_obj.uuid,
-                                   [{'path': '/cluster_template_id',
-                                     'op': 'remove'}],
-                                   expect_errors=True)
-        self.assertEqual(400, response.status_int)
-        self.assertEqual('application/json', response.content_type)
-        self.assertTrue(response.json['errors'])
+    def test_remove_mandatory_property_fail(self):
+        mandatory_properties = ('/uuid', '/cluster_template_id')
+        for p in mandatory_properties:
+            response = self.patch_json('/clusters/%s' % self.cluster_obj.uuid,
+                                       [{'path': p, 'op': 'remove'}],
+                                       expect_errors=True)
+            self.assertEqual(400, response.status_int)
+            self.assertEqual('application/json', response.content_type)
+            self.assertTrue(response.json['errors'])
 
     def test_remove_non_existent_property(self):
         response = self.patch_json(