From 51d59b9297b76e48ef9891b5afd81f049b153975 Mon Sep 17 00:00:00 2001
From: Anthony Lukach <alukach@cadasta.org>
Date: Thu, 5 Oct 2017 10:57:58 -0600
Subject: [PATCH] Fixup docstring, ensure no duplicate namespace

---
 cadasta/core/breakers/breaker.py    | 12 ++++++------
 cadasta/core/breakers/storages.py   | 23 ++++++++++++++++-------
 cadasta/core/tests/test_breakers.py | 12 ++++++++++--
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/cadasta/core/breakers/breaker.py b/cadasta/core/breakers/breaker.py
index 1c085654d..1407e6d29 100644
--- a/cadasta/core/breakers/breaker.py
+++ b/cadasta/core/breakers/breaker.py
@@ -7,12 +7,12 @@
 class CircuitBreaker(pybreaker.CircuitBreaker):
     def __init__(self, name, expected_errors=(), **kwargs):
         # Set defaults
-        kwargs['state_storage'] = (
-            kwargs.get('state_storage') or CacheStorage(name))
-        kwargs['listeners'] = (
-            kwargs.get('listeners') or [LogListener()])
-        kwargs['exclude'] = (
-            kwargs.get('exclude') or [KeyboardInterrupt])
+        if 'state_storage' not in kwargs:
+            kwargs['state_storage'] = CacheStorage(name)
+        if 'listeners' not in kwargs:
+            kwargs['listeners'] = [LogListener()]
+        if 'exclude' not in kwargs:
+            kwargs['exclude'] = [KeyboardInterrupt]
 
         self.name = name
         # Convenience attribute to allow codebase to easily catch and
diff --git a/cadasta/core/breakers/storages.py b/cadasta/core/breakers/storages.py
index e8f1d3f82..6cbd9ebf0 100644
--- a/cadasta/core/breakers/storages.py
+++ b/cadasta/core/breakers/storages.py
@@ -1,4 +1,5 @@
 from functools import partial
+import weakref
 
 from django.core.cache import cache
 import pybreaker
@@ -18,19 +19,27 @@ def get_offline_cache_errors():
 
 class CircuitBreakerCacheStorage(pybreaker.CircuitBreakerStorage):
     """
-    Defines the underlying storage for a circuit breaker - the underlying
-    implementation should be in a subclass that overrides the method this
-    class defines.
+    A cache-based storage for pybreaker. If cache retrieval fails (in the event
+    of an infrastructure failure) the storage defaults to its provided
+    fallback_state.
     """
     BASE_NAMESPACE = 'pybreaker'
+    __NAMESPACES = weakref.WeakValueDictionary()
 
     def __init__(self, namespace, fallback_state=pybreaker.STATE_CLOSED):
         """
-        Creates a new instance with the given `state` and `redis` object. The
-        redis object should be similar to pyredis' StrictRedis class. If there
-        are any connection issues with cache, the `fallback_circuit_state` is
-        used to determine the state of the circuit.
+        Creates a new instance with the given `namespace` and an optional
+        `fallback_state` object. The namespace is used to identify the circuit
+        breaker within the cache and therefore must be different from any other
+        circuit breaker namespaces. If there are any connection issues with
+        cache, the `fallback_circuit_state` is used to determine the state of
+        the circuit.
         """
+        assert namespace not in self.__NAMESPACES, (
+            "Attempt to create circuit breaker for already-used namespace "
+            "{!r}".format(namespace))
+        self.__NAMESPACES[namespace] = self
+
         super(CircuitBreakerCacheStorage, self).__init__(namespace)
         self._namespace_name = namespace
         self._fallback_state = fallback_state
diff --git a/cadasta/core/tests/test_breakers.py b/cadasta/core/tests/test_breakers.py
index 32338ee04..886cbc130 100644
--- a/cadasta/core/tests/test_breakers.py
+++ b/cadasta/core/tests/test_breakers.py
@@ -7,13 +7,16 @@
 
 
 class BreakersTest(TestCase):
+
     def test_expected_errors(self):
-        cb = breaker.CircuitBreaker('test', expected_errors=(AttributeError,))
+        cb = breaker.CircuitBreaker(
+            'test', expected_errors=(AttributeError,), state_storage=None)
         assert cb.expected_errors == (
             pybreaker.CircuitBreakerError, AttributeError)
 
     def test_repr(self):
-        assert str(breaker.CircuitBreaker('test')) == '<CircuitBreaker: test>'
+        cb = breaker.CircuitBreaker('test', state_storage=None)
+        assert str(cb) == '<CircuitBreaker: test>'
 
     def test_is_open(self):
         cb = breaker.CircuitBreaker(
@@ -108,6 +111,11 @@ def _get_storage():
     def test_namespace(self):
         assert self._get_storage().namespace == 'pybreaker:testStorage'
 
+    def test_duplicate_namespace(self):
+        x = self._get_storage()  # NOQA
+        with self.assertRaises(AssertionError):
+            self._get_storage()
+
     def test_state_namespace(self):
         assert self._get_storage()._state_namespace == (
             'pybreaker:testStorage:state')