region.description is optional and can be null

region.description is documented to be optional and should be treated as
optional.

A null description provided when creating indicates that the user
doesn't want a description stored. This is equivalent to not sending
a description property at all.

A null description provided when updating indicates that the user
doesn't want a description stored and any existing description should be
removed.

In both cases we treat this as an empty string because our database
schema requires a non-null value for description.

Closes-Bug: #1398165
Change-Id: I4a890414927ab5278861341b71a3e7fb324946a4

keystone/catalog/core.py

@@ -103,10 +103,12 @@ class Manager(manager.Manager):
             msg = _('Duplicate ID, %s.') % region_ref['id']
             raise exception.Conflict(type='region', details=msg)
 
-        # NOTE(lbragstad): The description column of the region database
-        # can not be null. So if the user doesn't pass in a description then
-        # set it to an empty string.
-        region_ref.setdefault('description', '')
+        # NOTE(lbragstad,dstanek): The description column of the region
+        # database cannot be null. So if the user doesn't pass in a
+        # description or passes in a null description then set it to an
+        # empty string.
+        if region_ref.get('description') is None:
+            region_ref['description'] = ''
         try:
             ret = self.driver.create_region(region_ref)
         except exception.NotFound:
@@ -124,6 +126,11 @@ class Manager(manager.Manager):
             raise exception.RegionNotFound(region_id=region_id)
 
     def update_region(self, region_id, region_ref, initiator=None):
+        # NOTE(lbragstad,dstanek): The description column of the region
+        # database cannot be null. So if the user passes in a null
+        # description set it to an empty string.
+        if 'description' in region_ref and region_ref['description'] is None:
+            region_ref['description'] = ''
         ref = self.driver.update_region(region_id, region_ref)
         notifications.Audit.updated(self._REGION, region_id, initiator)
         self.get_region.invalidate(self, region_id)

keystone/catalog/schema.py

@@ -14,7 +14,9 @@ from keystone.common.validation import parameter_types
 
 
 _region_properties = {
-    'description': parameter_types.description,
+    'description': {
+        'type': ['string', 'null'],
+    },
     # NOTE(lbragstad): Regions use ID differently. The user can specify the ID
     # or it will be generated automatically.
     'id': {

keystone/tests/unit/test_v3_catalog.py

@@ -154,7 +154,7 @@ class CatalogTestCase(test_v3.RestfulTestCase):
         ref2 = self.new_region_ref()
 
         del ref1['description']
-        del ref2['description']
+        ref2['description'] = None
 
         resp1 = self.post(
             '/regions',
@@ -224,6 +224,39 @@ class CatalogTestCase(test_v3.RestfulTestCase):
             body={'region': region})
         self.assertValidRegionResponse(r, region)
 
+    def test_update_region_without_description_keeps_original(self):
+        """Call ``PATCH /regions/{region_id}``."""
+        region_ref = self.new_region_ref()
+
+        resp = self.post('/regions', body={'region': region_ref},
+                         expected_status=201)
+
+        region_updates = {
+            # update with something that's not the description
+            'parent_region_id': self.region_id,
+        }
+        resp = self.patch('/regions/%s' % region_ref['id'],
+                          body={'region': region_updates},
+                          expected_status=200)
+
+        # NOTE(dstanek): Keystone should keep the original description.
+        self.assertEqual(region_ref['description'],
+                         resp.result['region']['description'])
+
+    def test_update_region_with_null_description(self):
+        """Call ``PATCH /regions/{region_id}``."""
+        region = self.new_region_ref()
+        del region['id']
+        region['description'] = None
+        r = self.patch('/regions/%(region_id)s' % {
+            'region_id': self.region_id},
+            body={'region': region})
+
+        # NOTE(dstanek): Keystone should turn the provided None value into
+        # an empty string before storing in the backend.
+        region['description'] = ''
+        self.assertValidRegionResponse(r, region)
+
     def test_delete_region(self):
         """Call ``DELETE /regions/{region_id}``."""