catch all and throw repositoryerror except 404 for retrieve

start updating tests

backdrop/core/repository.py

@@ -9,6 +9,8 @@
 
 logger = logging.getLogger(__name__)
 
+class RepositoryError(Exception):
+  pass
 
 class _Repository(object):
 
@@ -57,10 +59,13 @@ def __init__(self, stagecraft_url, stagecraft_token):
     def get_all(self):
         data_set_url = '{url}/data-sets'.format(url=self._stagecraft_url)
 
-        json_response = _get_json_url(
-            data_set_url, self._stagecraft_token) or '[]'
-        data_sets = _decode_json(json_response)
+        try:
+            json_response = _get_json_url(
+                data_set_url, self._stagecraft_token)
+        except requests.HTTPError as e:
+            raise RepositoryError 
 
+        data_sets = _decode_json(json_response)
         return [_make_bucket_config(data_set) for data_set in data_sets]
 
     def retrieve(self, name):
@@ -70,8 +75,14 @@ def retrieve(self, name):
                         url=self._stagecraft_url,
                         data_set_name=name))
 
-        data_set = _decode_json(_get_json_url(
-            data_set_url, self._stagecraft_token))
+        try:
+            data_set = _decode_json(_get_json_url(
+                data_set_url, self._stagecraft_token))
+        except requests.HTTPError as e:
+            if e.response.status_code == 404:
+                return None
+            raise RepositoryError 
+
         return _make_bucket_config(data_set)
 
     def get_bucket_for_query(self, data_group, data_type):
@@ -87,8 +98,13 @@ def get_bucket_for_query(self, data_group, data_type):
                             url=self._stagecraft_url,
                             data_group_name=data_group,
                             data_type_name=data_type))
-        json_response = _get_json_url(
-            data_set_url, self._stagecraft_token) or '[]'
+
+        try:
+            json_response = _get_json_url(
+                data_set_url, self._stagecraft_token)
+        except requests.HTTPError as e:
+            raise RepositoryError 
+
         data_sets = _decode_json(json_response)
         if len(data_sets) > 0:
             return _make_bucket_config(data_sets[0])
@@ -115,11 +131,9 @@ def _get_json_url(url, token):
     try:
         response.raise_for_status()
     except requests.HTTPError as e:
-        if e.response.status_code == 404:
-            logger.error('Got HTTP 404 for: {}'.format(url))
-            logger.exception(e)
-            return None
-        raise e
+        logger.error('Got HTTP {} for: {}'.format(response.status_code, url))
+        logger.exception(e)
+        raise
     return response.content
 
 

tests/core/test_repository.py

@@ -4,6 +4,7 @@
 from backdrop.core.bucket import BucketConfig
 from backdrop.core.repository import (BucketConfigRepository,
                                       UserConfigRepository,
+                                      RepositoryError,
                                       _get_json_url)
 from hamcrest import assert_that, equal_to, is_, has_entries, match_equality
 from mock import Mock, patch
@@ -37,22 +38,6 @@ def test_get_json_url_sends_correct_request(self, mock_get):
                      'Authorization': 'Bearer some_token'})
         assert_that(response_content, equal_to('[]'))
 
-    @patch('requests.get', spec=True)
-    def test_get_json_url_returns_none_on_404(self, mock_get):
-        exception = requests.HTTPError()
-        mock_error_response = Mock()
-        mock_error_response.status_code = 404
-        exception.response = mock_error_response
-
-        def fake_raise_for_status():
-            raise exception
-
-        mock_response = Mock()
-        mock_response.raise_for_status = fake_raise_for_status
-        mock_get.return_value = mock_response
-        response_content = _get_json_url("my_url", "some_token")
-        assert_that(response_content, is_(None))
-
 
 class TestBucketRepository(unittest.TestCase):
     def setUp(self):
@@ -128,22 +113,42 @@ def test_get_bucket_for_query_correctly_decodes_stagecraft_response(self):
 
             assert_that(bucket, equal_to(expected_bucket))
 
-    def test_retrieve_for_non_existent_bucket_returns_none(self):
-        with mock.patch(_GET_JSON_URL_FUNC) as _get_json_url:
-            _get_json_url.return_value = None
-            bucket = self.bucket_repo.retrieve(name="non_existent")
+    def test_retrieve_for_when_404_returns_none(self):
+        exception = requests.HTTPError()
+        mock_error_response = Mock()
+        mock_error_response.status_code = 404
+        exception.response = mock_error_response
 
+        def fake_get_json_url(*args):
+            raise exception
+
+        with mock.patch(_GET_JSON_URL_FUNC, new=fake_get_json_url) as _get_json_url:
+            bucket = self.bucket_repo.retrieve(name="non_existent")
+
         assert_that(bucket, is_(None))
 
-    def test_get_bucket_for_query_for_non_existent_bucket_returns_none(self):
+    def test_retrieve_catches_http_errors_and_throws_repository_error(self):
+        exception = requests.HTTPError()
+        mock_error_response = Mock()
+        mock_error_response.status_code = 500 
+        exception.response = mock_error_response
+
+        def fake_get_json_url(*args):
+            raise exception
+
+        with mock.patch(_GET_JSON_URL_FUNC, new=fake_get_json_url) as _get_json_url:
+            with self.assertRaises(RepositoryError):
+                bucket = self.bucket_repo.retrieve(name="non_existent")
+
+    def test_get_bucket_for_query_catches_http_errors_and_throws_repository_error(self):
         with mock.patch(_GET_JSON_URL_FUNC) as _get_json_url:
             _get_json_url.return_value = None
             bucket = self.bucket_repo.get_bucket_for_query(
                 data_group="govuk", data_type="realtime")
 
         assert_that(bucket, is_(None))
 
-    def test_get_all_for_non_existent_buckets_returns_empty_array(self):
+    def test_get_all_catches_http_errors_and_throws_repository_error(self):
         with mock.patch(_GET_JSON_URL_FUNC) as _get_json_url:
             _get_json_url.return_value = None
             buckets = self.bucket_repo.get_all()