From b17dd7ec75eaa4af6db62dd55f8a94d4c9f972bc Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Wed, 24 Mar 2021 12:00:55 +0000
Subject: [PATCH] Make ShardRange.OuterBound a proper singleton

ShardRange.OuterBound had the singleton-like property of all instances
of the same class being equal, but did not prevent multiple instances
being created.  Instead, ShardRange.MIN and ShardRange.MAX are used to
hold single instances of each outer bound.

The ShardRange.OuterBound equality property was achieved by overriding
the default __eq__ method, but as a consequence the type was not
hashable, preventing it being used in lists or as a dict key without
also overriding the default __hash__ method.

This patch makes ShardRangeOuterBound a proper singleton, removing the
need to override __eq__ or __hash__.

Change-Id: I21292e7991e93834b35cda6f5daea4c552a8e999
---
 swift/common/utils.py          | 49 +++++++++++++++++++---------------
 test/unit/common/test_utils.py | 17 ++++++++++++
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/swift/common/utils.py b/swift/common/utils.py
index 8174532c86..e4c2bccf3c 100644
--- a/swift/common/utils.py
+++ b/swift/common/utils.py
@@ -4970,6 +4970,29 @@ except TypeError:
         return hashlib.md5(string)  # nosec
 
 
+class ShardRangeOuterBound(object):
+    """
+    A custom singleton type used for the outer bounds of ShardRanges.
+    """
+    _singleton = None
+
+    def __new__(cls):
+        if cls._singleton is None:
+            cls._singleton = super(ShardRangeOuterBound, cls).__new__(cls)
+        return cls._singleton
+
+    def __str__(self):
+        return ''
+
+    def __repr__(self):
+        return type(self).__name__
+
+    def __bool__(self):
+        return False
+
+    __nonzero__ = __bool__
+
+
 class ShardRange(object):
     """
     A ShardRange encapsulates sharding state related to a container including
@@ -5034,31 +5057,15 @@ class ShardRange(object):
               SHRUNK: 'shrunk'}
     STATES_BY_NAME = dict((v, k) for k, v in STATES.items())
 
-    class OuterBound(object):
-        def __eq__(self, other):
-            return isinstance(other, type(self))
-
-        def __ne__(self, other):
-            return not self.__eq__(other)
-
-        def __str__(self):
-            return ''
-
-        def __repr__(self):
-            return type(self).__name__
-
-        def __bool__(self):
-            return False
-
-        __nonzero__ = __bool__
-
     @functools.total_ordering
-    class MaxBound(OuterBound):
+    class MaxBound(ShardRangeOuterBound):
+        # singleton for maximum bound
         def __ge__(self, other):
             return True
 
     @functools.total_ordering
-    class MinBound(OuterBound):
+    class MinBound(ShardRangeOuterBound):
+        # singleton for minimum bound
         def __le__(self, other):
             return True
 
@@ -5107,7 +5114,7 @@ class ShardRange(object):
         return value
 
     def _encode_bound(self, bound):
-        if isinstance(bound, ShardRange.OuterBound):
+        if isinstance(bound, ShardRangeOuterBound):
             return bound
         if not (isinstance(bound, six.text_type) or
                 isinstance(bound, six.binary_type)):
diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py
index 180a21fc38..7bcf9f996d 100644
--- a/test/unit/common/test_utils.py
+++ b/test/unit/common/test_utils.py
@@ -7750,6 +7750,10 @@ class TestShardRange(unittest.TestCase):
         self.assertFalse(utils.ShardRange.MAX != utils.ShardRange.MAX)
         self.assertTrue(
             utils.ShardRange.MaxBound() == utils.ShardRange.MaxBound())
+        self.assertTrue(
+            utils.ShardRange.MaxBound() is utils.ShardRange.MaxBound())
+        self.assertTrue(
+            utils.ShardRange.MaxBound() is utils.ShardRange.MAX)
         self.assertFalse(
             utils.ShardRange.MaxBound() != utils.ShardRange.MaxBound())
 
@@ -7772,6 +7776,10 @@ class TestShardRange(unittest.TestCase):
         self.assertFalse(utils.ShardRange.MIN != utils.ShardRange.MIN)
         self.assertTrue(
             utils.ShardRange.MinBound() == utils.ShardRange.MinBound())
+        self.assertTrue(
+            utils.ShardRange.MinBound() is utils.ShardRange.MinBound())
+        self.assertTrue(
+            utils.ShardRange.MinBound() is utils.ShardRange.MIN)
         self.assertFalse(
             utils.ShardRange.MinBound() != utils.ShardRange.MinBound())
 
@@ -7779,12 +7787,21 @@ class TestShardRange(unittest.TestCase):
         self.assertFalse(utils.ShardRange.MIN == utils.ShardRange.MAX)
         self.assertTrue(utils.ShardRange.MAX != utils.ShardRange.MIN)
         self.assertTrue(utils.ShardRange.MIN != utils.ShardRange.MAX)
+        self.assertFalse(utils.ShardRange.MAX is utils.ShardRange.MIN)
 
         self.assertEqual(utils.ShardRange.MAX,
                          max(utils.ShardRange.MIN, utils.ShardRange.MAX))
         self.assertEqual(utils.ShardRange.MIN,
                          min(utils.ShardRange.MIN, utils.ShardRange.MAX))
 
+        # check the outer bounds are hashable
+        hashmap = {utils.ShardRange.MIN: 'min',
+                   utils.ShardRange.MAX: 'max'}
+        self.assertEqual(hashmap[utils.ShardRange.MIN], 'min')
+        self.assertEqual(hashmap[utils.ShardRange.MinBound()], 'min')
+        self.assertEqual(hashmap[utils.ShardRange.MAX], 'max')
+        self.assertEqual(hashmap[utils.ShardRange.MaxBound()], 'max')
+
     def test_shard_range_initialisation(self):
         def assert_initialisation_ok(params, expected):
             pr = utils.ShardRange(**params)