move (non-api) clients (inc redis) from app/__init__.py to extensions

when clients are defined in app/__init__.py, it increases the chance of 
cyclical imports. By moving module level client singletons out to a 
separate extensions file, we stop cyclical imports, but keep the same 
code flow - the clients are still initialised in `create_app` in 
`__init__.py`.

The redis client in particular is no longer separate - previously redis 
was set up on the `NotifyAdminAPIClient` base class, but now there's one 
singleton in `app.extensions`. This was done so that we can access redis 
from outside of the existing clients.

app/__init__.py

@@ -25,9 +25,6 @@ from functools import partial
 
 from notifications_python_client.errors import HTTPError
 from notifications_utils import logging, request_helper, formatters
-from notifications_utils.clients.antivirus.antivirus_client import AntivirusClient
-from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
-from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.recipients import (
     validate_phone_number,
     InvalidPhoneError,
@@ -42,6 +39,12 @@ from werkzeug.local import LocalProxy
 from app import proxy_fix
 from app.config import configs
 from app.asset_fingerprinter import asset_fingerprinter
+from app.extensions import (
+    antivirus_client,
+    statsd_client,
+    zendesk_client,
+    redis_client,
+)
 from app.models.service import Service
 from app.models.user import AnonymousUser
 from app.navigation import (
@@ -75,9 +78,6 @@ from app.utils import get_logo_cdn_domain, id_safe
 
 login_manager = LoginManager()
 csrf = CSRFProtect()
-antivirus_client = AntivirusClient()
-statsd_client = StatsdClient()
-zendesk_client = ZendeskClient()
 
 
 # The current service attached to the request stack.
@@ -116,8 +116,7 @@ def create_app(application):
         proxy_fix,
         request_helper,
 
-        # Internal API clients
-        antivirus_client,
+        # API clients
         api_key_api_client,
         billing_api_client,
         complaint_api_client,
@@ -140,8 +139,10 @@ def create_app(application):
         user_api_client,
 
         # External API clients
+        antivirus_client,
         statsd_client,
         zendesk_client,
+        redis_client
 
     ):
         client.init_app(application)
@@ -153,6 +154,9 @@ def create_app(application):
     login_manager.session_protection = None
     login_manager.anonymous_user = AnonymousUser
 
+    # make sure we handle unicode correctly
+    redis_client.redis_store.decode_responses = True
+
     from app.main import main as main_blueprint
     application.register_blueprint(main_blueprint)
 

app/extensions.py

@@ -0,0 +1,11 @@
+from notifications_utils.clients.antivirus.antivirus_client import (
+    AntivirusClient,
+)
+from notifications_utils.clients.redis.redis_client import RedisClient
+from notifications_utils.clients.statsd.statsd_client import StatsdClient
+from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
+
+antivirus_client = AntivirusClient()
+statsd_client = StatsdClient()
+zendesk_client = ZendeskClient()
+redis_client = RedisClient()

app/main/views/feedback.py

@@ -4,12 +4,8 @@ import pytz
 from flask import abort, redirect, render_template, request, session, url_for
 from flask_login import current_user
 
-from app import (
-    convert_to_boolean,
-    current_service,
-    service_api_client,
-    zendesk_client,
-)
+from app import convert_to_boolean, current_service, service_api_client
+from app.extensions import zendesk_client
 from app.main import main
 from app.main.forms import Feedback, Problem, SupportType, Triage
 

app/main/views/platform_admin.py

@@ -8,12 +8,12 @@ from notifications_python_client.errors import HTTPError
 from requests import RequestException
 
 from app import (
-    antivirus_client,
     complaint_api_client,
     letter_jobs_client,
     platform_stats_api_client,
     service_api_client,
 )
+from app.extensions import antivirus_client
 from app.main import main
 from app.main.forms import DateFilterForm, PDFUploadForm, ReturnedLettersForm
 from app.statistics_utils import (

app/main/views/service_settings.py

@@ -23,8 +23,8 @@ from app import (
     organisations_client,
     service_api_client,
     user_api_client,
-    zendesk_client,
 )
+from app.extensions import zendesk_client
 from app.main import main
 from app.main.forms import (
     BrandingOptionsEmail,

app/notify_client/__init__.py

@@ -2,7 +2,6 @@ from flask_login import current_user
 from flask import has_request_context, request, abort
 from notifications_python_client.base import BaseAPIClient
 from notifications_python_client import __version__
-from notifications_utils.clients.redis.redis_client import RedisClient
 
 
 def _attach_current_user(data):
@@ -14,8 +13,6 @@ def _attach_current_user(data):
 
 class NotifyAdminAPIClient(BaseAPIClient):
 
-    redis_client = RedisClient()
-
     def __init__(self):
         super().__init__("a" * 73, "b")
 
@@ -24,8 +21,6 @@ class NotifyAdminAPIClient(BaseAPIClient):
         self.service_id = app.config['ADMIN_CLIENT_USER_NAME']
         self.api_key = app.config['ADMIN_CLIENT_SECRET']
         self.route_secret = app.config['ROUTE_SECRET_KEY_1']
-        self.redis_client.init_app(app)
-        self.redis_client.redis_store.decode_responses = True
 
     def generate_headers(self, api_token):
         headers = {

app/notify_client/cache.py

@@ -4,6 +4,8 @@ from datetime import timedelta
 from functools import wraps
 from inspect import signature
 
+from app.extensions import redis_client
+
 TTL = int(timedelta(days=7).total_seconds())
 
 
@@ -38,11 +40,11 @@ def set(key_format):
         @wraps(client_method)
         def new_client_method(client_instance, *args, **kwargs):
             redis_key = _make_key(key_format, client_method, args, kwargs)
-            cached = client_instance.redis_client.get(redis_key)
+            cached = redis_client.get(redis_key)
             if cached:
                 return json.loads(cached.decode('utf-8'))
             api_response = client_method(client_instance, *args, **kwargs)
-            client_instance.redis_client.set(
+            redis_client.set(
                 redis_key,
                 json.dumps(api_response),
                 ex=TTL,
@@ -60,7 +62,7 @@ def delete(key_format):
         @wraps(client_method)
         def new_client_method(client_instance, *args, **kwargs):
             redis_key = _make_key(key_format, client_method, args, kwargs)
-            client_instance.redis_client.delete(redis_key)
+            redis_client.delete(redis_key)
             return client_method(client_instance, *args, **kwargs)
 
         return new_client_method

app/notify_client/job_api_client.py

@@ -1,5 +1,6 @@
 from collections import defaultdict
 
+from app.extensions import redis_client
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
 
 
@@ -96,8 +97,7 @@ class JobApiClient(NotifyAdminAPIClient):
         return bool(self.get_jobs(service_id)['data'])
 
     def create_job(self, job_id, service_id, scheduled_for=None):
-
-        self.redis_client.set(
+        redis_client.set(
             'has_jobs-{}'.format(service_id),
             b'true',
             ex=cache.TTL,

app/notify_client/template_folder_api_client.py

@@ -1,3 +1,4 @@
+from app.extensions import redis_client
 from app.notify_client import NotifyAdminAPIClient, cache
 
 
@@ -35,7 +36,7 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient):
         })
 
         if template_ids:
-            self.redis_client.delete(*map(
+            redis_client.delete(*map(
                 'template-{}-version-None'.format,
                 template_ids,
             ))

tests/app/notify_client/test_email_branding_client.py

@@ -9,11 +9,11 @@ def test_get_email_branding(mocker, fake_uuid):
         return_value={'foo': 'bar'}
     )
     mock_redis_get = mocker.patch(
-        'app.notify_client.RedisClient.get',
+        'app.extensions.RedisClient.get',
         return_value=None,
     )
     mock_redis_set = mocker.patch(
-        'app.notify_client.RedisClient.set',
+        'app.extensions.RedisClient.set',
     )
     EmailBrandingClient().get_email_branding(fake_uuid)
     mock_get.assert_called_once_with(
@@ -33,11 +33,11 @@ def test_get_all_email_branding(mocker):
         return_value={'email_branding': [1, 2, 3]}
     )
     mock_redis_get = mocker.patch(
-        'app.notify_client.RedisClient.get',
+        'app.extensions.RedisClient.get',
         return_value=None,
     )
     mock_redis_set = mocker.patch(
-        'app.notify_client.RedisClient.set',
+        'app.extensions.RedisClient.set',
     )
     EmailBrandingClient().get_all_email_branding()
     mock_get.assert_called_once_with(
@@ -56,7 +56,7 @@ def test_create_email_branding(mocker):
                 'domain': 'sample.com', 'brand_type': 'org'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     EmailBrandingClient().create_email_branding(
         logo=org_data['logo'], name=org_data['name'], text=org_data['text'], colour=org_data['colour'],
         domain=org_data['domain'], brand_type='org'
@@ -75,7 +75,7 @@ def test_update_email_branding(mocker, fake_uuid):
                 'domain': 'sample.com', 'brand_type': 'org'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     EmailBrandingClient().update_email_branding(
         branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], text=org_data['text'],
         colour=org_data['colour'], domain=org_data['domain'], brand_type='org')

tests/app/notify_client/test_job_client.py

@@ -10,7 +10,7 @@ def test_client_creates_job_data_correctly(mocker, fake_uuid):
     job_id = fake_uuid
     service_id = fake_uuid
     mocker.patch('app.notify_client.current_user', id='1')
-    mock_redis_set = mocker.patch('app.notify_client.RedisClient.set')
+    mock_redis_set = mocker.patch('app.extensions.RedisClient.set')
 
     expected_data = {
         "id": job_id,
@@ -330,7 +330,7 @@ def test_has_jobs_sets_cache(
         'app.notify_client.job_api_client.JobApiClient.get',
         return_value={'data': job_data}
     )
-    mock_redis_set = mocker.patch('app.notify_client.RedisClient.set')
+    mock_redis_set = mocker.patch('app.extensions.RedisClient.set')
 
     JobApiClient().has_jobs(fake_uuid)
 
@@ -359,7 +359,7 @@ def test_has_jobs_returns_from_cache(
         'app.notify_client.job_api_client.JobApiClient.get'
     )
     mock_redis_get = mocker.patch(
-        'app.notify_client.RedisClient.get',
+        'app.extensions.RedisClient.get',
         return_value=cache_value,
     )
 

tests/app/notify_client/test_letter_branding_client.py

@@ -8,8 +8,8 @@ def test_get_letter_branding(mocker, fake_uuid):
         'app.notify_client.letter_branding_client.LetterBrandingClient.get',
         return_value={'foo': 'bar'}
     )
-    mock_redis_get = mocker.patch('app.notify_client.RedisClient.get', return_value=None)
-    mock_redis_set = mocker.patch('app.notify_client.RedisClient.set')
+    mock_redis_get = mocker.patch('app.extensions.RedisClient.get', return_value=None)
+    mock_redis_set = mocker.patch('app.extensions.RedisClient.set')
 
     LetterBrandingClient().get_letter_branding(fake_uuid)
 
@@ -24,8 +24,8 @@ def test_get_letter_branding(mocker, fake_uuid):
 
 def test_get_all_letter_branding(mocker):
     mock_get = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.get', return_value=[1, 2, 3])
-    mock_redis_get = mocker.patch('app.notify_client.RedisClient.get', return_value=None)
-    mock_redis_set = mocker.patch('app.notify_client.RedisClient.set')
+    mock_redis_get = mocker.patch('app.extensions.RedisClient.get', return_value=None)
+    mock_redis_set = mocker.patch('app.extensions.RedisClient.set')
 
     LetterBrandingClient().get_all_letter_branding()
 
@@ -42,7 +42,7 @@ def test_create_letter_branding(mocker):
     new_branding = {'filename': 'uuid-test', 'name': 'my letters', 'domain': 'example.com'}
 
     mock_post = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.post')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
 
     LetterBrandingClient().create_letter_branding(
         filename=new_branding['filename'], name=new_branding['name'], domain=new_branding['domain']
@@ -59,7 +59,7 @@ def test_update_letter_branding(mocker, fake_uuid):
     branding = {'filename': 'uuid-test', 'name': 'my letters', 'domain': 'example.com'}
 
     mock_post = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.post')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     LetterBrandingClient().update_letter_branding(
         branding_id=fake_uuid, filename=branding['filename'], name=branding['name'], domain=branding['domain'])
 

tests/app/notify_client/test_service_api_client.py

@@ -324,7 +324,7 @@ def test_returns_value_from_cache(
 ):
 
     mock_redis_get = mocker.patch(
-        'app.notify_client.RedisClient.get',
+        'app.extensions.RedisClient.get',
         return_value=cache_value,
     )
     mock_api_get = mocker.patch(
@@ -332,7 +332,7 @@ def test_returns_value_from_cache(
         return_value={'data_from': 'api'},
     )
     mock_redis_set = mocker.patch(
-        'app.notify_client.RedisClient.set',
+        'app.extensions.RedisClient.set',
     )
 
     assert client_method(*extra_args) == expected_return_value
@@ -375,7 +375,7 @@ def test_deletes_service_cache(
     extra_kwargs,
 ):
     mocker.patch('app.notify_client.current_user', id='1')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     mock_request = mocker.patch('notifications_python_client.base.BaseAPIClient.request')
 
     getattr(client, method)(*extra_args, **extra_kwargs)
@@ -423,7 +423,7 @@ def test_deletes_caches_when_modifying_templates(
     expected_cache_deletes,
 ):
     mocker.patch('app.notify_client.current_user', id='1')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     mock_request = mocker.patch('notifications_python_client.base.BaseAPIClient.request')
 
     getattr(service_api_client, method)(*extra_args)

tests/app/notify_client/test_template_folder_client.py

@@ -9,7 +9,7 @@ from app.notify_client.template_folder_api_client import TemplateFolderAPIClient
 
 @pytest.mark.parametrize('parent_id', [uuid.uuid4(), None])
 def test_create_template_folder_calls_correct_api_endpoint(mocker, parent_id):
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
 
     some_service_id = uuid.uuid4()
     expected_url = '/service/{}/template-folder'.format(some_service_id)
@@ -26,8 +26,8 @@ def test_create_template_folder_calls_correct_api_endpoint(mocker, parent_id):
 
 
 def test_get_template_folders_calls_correct_api_endpoint(mocker):
-    mock_redis_get = mocker.patch('app.notify_client.RedisClient.get', return_value=None)
-    mock_redis_set = mocker.patch('app.notify_client.RedisClient.set')
+    mock_redis_get = mocker.patch('app.extensions.RedisClient.get', return_value=None)
+    mock_redis_set = mocker.patch('app.extensions.RedisClient.set')
     mock_api_get = mocker.patch(
         'app.notify_client.NotifyAdminAPIClient.get',
         return_value={'template_folders': {'a': 'b'}}
@@ -50,7 +50,7 @@ def test_get_template_folders_calls_correct_api_endpoint(mocker):
 
 def test_move_templates_and_folders(mocker):
 
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     mock_api_post = mocker.patch('app.notify_client.NotifyAdminAPIClient.post')
 
     some_service_id = uuid.uuid4()
@@ -106,7 +106,7 @@ def test_move_templates_and_folders_to_root(mocker):
 
 
 def test_update_template_folder_calls_correct_api_endpoint(mocker):
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
 
     some_service_id = uuid.uuid4()
     template_folder_id = uuid.uuid4()
@@ -124,7 +124,7 @@ def test_update_template_folder_calls_correct_api_endpoint(mocker):
 
 
 def test_delete_template_folder_calls_correct_api_endpoint(mocker):
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
 
     some_service_id = uuid.uuid4()
     template_folder_id = uuid.uuid4()

tests/app/notify_client/test_user_client.py

@@ -210,7 +210,7 @@ def test_returns_value_from_cache(
 ):
 
     mock_redis_get = mocker.patch(
-        'app.notify_client.RedisClient.get',
+        'app.extensions.RedisClient.get',
         return_value=cache_value,
     )
     mock_api_get = mocker.patch(
@@ -218,7 +218,7 @@ def test_returns_value_from_cache(
         return_value={'data': 'from api'},
     )
     mock_redis_set = mocker.patch(
-        'app.notify_client.RedisClient.set',
+        'app.extensions.RedisClient.set',
     )
     mock_model = mocker.patch(
         'app.models.user.User.__init__',
@@ -262,7 +262,7 @@ def test_deletes_user_cache(
     extra_kwargs,
 ):
     mocker.patch('app.notify_client.current_user', id='1')
-    mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete')
+    mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     mock_request = mocker.patch('notifications_python_client.base.BaseAPIClient.request')
 
     getattr(client, method)(*extra_args, **extra_kwargs)