Utils: fix Namespace and ShardRange attribute encoding in py2.

Ensure name/account/container are always consistent and always encode
utf8 in py2.

Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
Change-Id: Ia5374f55adf80fef92a92d916b3f89297463c673

swift/common/utils/__init__.py

@@ -4546,7 +4546,7 @@ class Namespace(object):
     :param upper: the upper bound of object names contained in the namespace;
         the upper bound *is* included in the namespace.
     """
-    __slots__ = ('_lower', '_upper', 'name')
+    __slots__ = ('_lower', '_upper', '_name')
 
     @functools.total_ordering
     class MaxBound(NamespaceOuterBound):
@@ -4641,6 +4641,14 @@ class Namespace(object):
             raise TypeError('must be a string type')
         return self._encode(bound)
 
+    @property
+    def name(self):
+        return self._name
+
+    @name.setter
+    def name(self, path):
+        self._name = self._encode(path)
+
     @property
     def lower(self):
         return self._lower
@@ -5024,7 +5032,7 @@ class ShardRange(Namespace):
     CLEAVING_STATES = SHRINKING_STATES + SHARDING_STATES
 
     __slots__ = (
-        'account', 'container',
+        '_account', '_container',
         '_timestamp', '_meta_timestamp', '_state_timestamp', '_epoch',
         '_deleted', '_state', '_count', '_bytes',
         '_tombstones', '_reported')
@@ -5034,13 +5042,13 @@ class ShardRange(Namespace):
                  object_count=0, bytes_used=0, meta_timestamp=None,
                  deleted=False, state=None, state_timestamp=None, epoch=None,
                  reported=False, tombstones=-1, **kwargs):
+        self._account = self._container = None
         super(ShardRange, self).__init__(name=name, lower=lower, upper=upper)
-        self.account = self.container = self._timestamp = \
+        self._timestamp = self._meta_timestamp = self._state_timestamp = \
-            self._meta_timestamp = self._state_timestamp = self._epoch = None
+            self._epoch = None
         self._deleted = False
         self._state = None
 
-        self.name = name
         self.timestamp = timestamp
         self.deleted = deleted
         self.object_count = object_count
@@ -5197,18 +5205,34 @@ class ShardRange(Namespace):
             return timestamp
         return Timestamp(timestamp)
 
+    @property
+    def account(self):
+        return self._account
+
+    @account.setter
+    def account(self, value):
+        self._account = self._encode(value)
+
+    @property
+    def container(self):
+        return self._container
+
+    @container.setter
+    def container(self, value):
+        self._container = self._encode(value)
+
     @property
     def name(self):
         return '%s/%s' % (self.account, self.container)
 
     @name.setter
-    def name(self, path):
+    def name(self, name):
-        path = self._encode(path)
+        name = self._encode(name)
-        if not path or len(path.split('/')) != 2 or not all(path.split('/')):
+        if not name or len(name.split('/')) != 2 or not all(name.split('/')):
             raise ValueError(
                 "Name must be of the form '<account>/<container>', got %r" %
-                path)
+                name)
-        self.account, self.container = path.split('/')
+        self._account, self._container = name.split('/')
 
     @property
     def timestamp(self):

test/unit/common/test_utils.py

@@ -7456,6 +7456,23 @@ class TestNamespace(unittest.TestCase):
         self.assertEqual(exp_upper, ns.upper_str)
         self.assertEqual(exp_upper + '\x00', ns.end_marker)
 
+    def test_unicode_name(self):
+        shard_bounds = ('', 'ham', 'pie', u'\N{SNOWMAN}', u'\U0001F334', '')
+        bounds = [(l, u) for l, u in zip(shard_bounds[:-1], shard_bounds[1:])]
+        namespaces = [utils.Namespace('.shards_a/c_%s' % upper, lower, upper)
+                      for lower, upper in bounds]
+        if six.PY2:
+            exp_bounds = [(l.encode('utf8'), u.encode('utf8'))
+                          for l, u in bounds]
+        else:
+            exp_bounds = bounds
+        for i in range(len(exp_bounds)):
+            self.assertEqual(namespaces[i].name,
+                             '.shards_a/c_%s' % exp_bounds[i][1])
+            self.assertEqual(namespaces[i].lower_str, exp_bounds[i][0])
+
+            self.assertEqual(namespaces[i].upper_str, exp_bounds[i][1])
+
     def test_entire_namespace(self):
         # test entire range (no boundaries)
         entire = utils.Namespace('a/test', None, None)
@@ -8027,6 +8044,55 @@ class TestShardRange(unittest.TestCase):
         self._check_to_from_dict('l', 'u')
         self._check_to_from_dict('', '')
 
+    def _check_name_account_container(self, sr, exp_name):
+        # check that the name, account, container properties are consistent
+        exp_account, exp_container = exp_name.split('/')
+        if six.PY2:
+            self.assertEqual(exp_name.encode('utf8'), sr.name)
+            self.assertEqual(exp_account.encode('utf8'), sr.account)
+            self.assertEqual(exp_container.encode('utf8'), sr.container)
+        else:
+            self.assertEqual(exp_name, sr.name)
+            self.assertEqual(exp_account, sr.account)
+            self.assertEqual(exp_container, sr.container)
+
+    def test_name(self):
+        # constructor
+        path = 'a/c'
+        sr = utils.ShardRange(path, 0, 'l', 'u')
+        self._check_name_account_container(sr, path)
+        # name setter
+        path = 'a2/c2'
+        sr.name = path
+        self._check_name_account_container(sr, path)
+
+        # constructor
+        path = u'\u1234a/\n{SNOWMAN}'
+        sr = utils.ShardRange(path, 0, 'l', 'u')
+        self._check_name_account_container(sr, path)
+        # name setter
+        path = u'\n{SNOWMAN}/\u1234c'
+        sr.name = path
+        self._check_name_account_container(sr, path)
+
+    def test_account(self):
+        path = 'a/c'
+        sr = utils.ShardRange(path, 0, 'l', 'u')
+        self._check_name_account_container(sr, path)
+        sr.account = 'a2'
+        self._check_name_account_container(sr, 'a2/c')
+        sr.account = u'\n{SNOWMAN}'
+        self._check_name_account_container(sr, u'\n{SNOWMAN}/c')
+
+    def test_container(self):
+        path = 'a/c'
+        sr = utils.ShardRange(path, 0, 'l', 'u')
+        self._check_name_account_container(sr, path)
+        sr.container = 'c2'
+        self._check_name_account_container(sr, 'a/c2')
+        sr.container = u'\n{SNOWMAN}'
+        self._check_name_account_container(sr, u'a/\n{SNOWMAN}')
+
     def test_timestamp_setter(self):
         ts_1 = next(self.ts_iter)
         sr = utils.ShardRange('a/test', ts_1, 'l', 'u', 0, 0, None)