Merge pull request #1107 from GSA/fix_raw_set_and_get

fix raw_set and raw_get

app/delivery/send_to_providers.py

@@ -89,7 +89,7 @@ def send_sms_to_provider(notification):
                 except Exception:
                     # It is our 2facode, maybe
                     key = f"2facode-{notification.id}".replace(" ", "")
-                    recipient = redis_store.raw_get(key)
+                    recipient = redis_store.get(key)
 
                     if recipient:
                         recipient = recipient.decode("utf-8")

app/user/rest.py

@@ -361,7 +361,7 @@ def create_2fa_code(
     saved_notification.personalisation = personalisation
     key = f"2facode-{saved_notification.id}".replace(" ", "")
     recipient = str(recipient)
-    redis_store.raw_set(key, recipient, ex=60 * 60)
+    redis_store.set(key, recipient, ex=60 * 60)
 
     # Assume that we never want to observe the Notify service's research mode
     # setting for this notification - we still need to be able to log into the

notifications_utils/clients/redis/redis_client.py

@@ -133,19 +133,13 @@ class RedisClient:
         else:
             return False
 
-    def raw_set(self, key, value, ex=None, px=None, nx=False, xx=False):
-        self.redis_store.set(key, value, ex, px, nx, xx)
-
     def set(
         self, key, value, ex=None, px=None, nx=False, xx=False, raise_exception=False
     ):
         key = prepare_value(key)
         value = prepare_value(value)
         if self.active:
-            try:
-                self.redis_store.set(key, value, ex, px, nx, xx)
-            except Exception as e:
-                self.__handle_exception(e, raise_exception, "set", key)
+            self.redis_store.set(key, value, ex, px, nx, xx)
 
     def incr(self, key, raise_exception=False):
         key = prepare_value(key)
@@ -155,16 +149,10 @@ class RedisClient:
             except Exception as e:
                 self.__handle_exception(e, raise_exception, "incr", key)
 
-    def raw_get(self, key):
-        return self.redis_store.get(key)
-
     def get(self, key, raise_exception=False):
         key = prepare_value(key)
         if self.active:
-            try:
-                return self.redis_store.get(key)
-            except Exception as e:
-                self.__handle_exception(e, raise_exception, "get", key)
+            return self.redis_store.get(key)
 
         return None
 

tests/app/service_invite/test_service_invite_rest.py

@@ -31,8 +31,8 @@ def test_create_invited_user(
     extra_args,
     expected_start_of_invite_url,
 ):
-    mocker.patch("app.service_invite.rest.redis_store.raw_set")
-    mocker.patch("app.service_invite.rest.redis_store.raw_get")
+    mocker.patch("app.service_invite.rest.redis_store.set")
+    mocker.patch("app.service_invite.rest.redis_store.get")
 
     mocked = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async")
     email_address = "invited_user@service.gov.uk"
@@ -96,8 +96,8 @@ def test_create_invited_user_without_auth_type(
     admin_request, sample_service, mocker, invitation_email_template
 ):
 
-    mocker.patch("app.service_invite.rest.redis_store.raw_set")
-    mocker.patch("app.service_invite.rest.redis_store.raw_get")
+    mocker.patch("app.service_invite.rest.redis_store.set")
+    mocker.patch("app.service_invite.rest.redis_store.get")
     mocker.patch("app.celery.provider_tasks.deliver_email.apply_async")
     email_address = "invited_user@service.gov.uk"
     invite_from = sample_service.users[0]
@@ -220,8 +220,8 @@ def test_resend_expired_invite(
     mocker,
 ):
 
-    mocker.patch("app.service_invite.rest.redis_store.raw_set")
-    mocker.patch("app.service_invite.rest.redis_store.raw_get")
+    mocker.patch("app.service_invite.rest.redis_store.set")
+    mocker.patch("app.service_invite.rest.redis_store.get")
     url = f"/service/{sample_expired_user.service_id}/invite/{sample_expired_user.id}/resend"
     mock_send = mocker.patch("app.service_invite.rest.send_notification_to_queue")
     mock_persist = mocker.patch("app.service_invite.rest.persist_notification")

tests/app/user/test_rest_verify.py

@@ -201,10 +201,10 @@ def test_send_user_sms_code(client, sample_user, sms_code_template, mocker):
     """
     notify_service = dao_fetch_service_by_id(current_app.config["NOTIFY_SERVICE_ID"])
 
-    mock_redis_get = mocker.patch("app.user.rest.redis_store.raw_get")
+    mock_redis_get = mocker.patch("app.user.rest.redis_store.get")
     mock_redis_get.return_value = "foo"
 
-    mocker.patch("app.user.rest.redis_store.raw_set")
+    mocker.patch("app.user.rest.redis_store.set")
     auth_header = create_admin_authorization_header()
     mocked = mocker.patch("app.user.rest.create_secret_code", return_value="11111")
     mocker.patch("app.celery.provider_tasks.deliver_sms.apply_async")
@@ -242,10 +242,10 @@ def test_send_user_code_for_sms_with_optional_to_field(
     Tests POST endpoint /user/<user_id>/sms-code with optional to field
     """
 
-    mock_redis_get = mocker.patch("app.user.rest.redis_store.raw_get")
+    mock_redis_get = mocker.patch("app.user.rest.redis_store.get")
     mock_redis_get.return_value = "foo"
 
-    mocker.patch("app.user.rest.redis_store.raw_set")
+    mocker.patch("app.user.rest.redis_store.set")
     to_number = "+14254147755"
     mocked = mocker.patch("app.user.rest.create_secret_code", return_value="11111")
     mocker.patch("app.celery.provider_tasks.deliver_sms.apply_async")
@@ -469,10 +469,10 @@ def test_send_user_email_code(
     deliver_email = mocker.patch("app.celery.provider_tasks.deliver_email.apply_async")
     sample_user.auth_type = auth_type
 
-    mock_redis_get = mocker.patch("app.user.rest.redis_store.raw_get")
+    mock_redis_get = mocker.patch("app.user.rest.redis_store.get")
     mock_redis_get.return_value = "foo"
 
-    mocker.patch("app.user.rest.redis_store.raw_set")
+    mocker.patch("app.user.rest.redis_store.set")
 
     admin_request.post(
         "user.send_user_2fa_code",
@@ -497,10 +497,10 @@ def test_send_user_email_code_with_urlencoded_next_param(
 ):
     mocker.patch("app.celery.provider_tasks.deliver_email.apply_async")
 
-    mock_redis_get = mocker.patch("app.celery.scheduled_tasks.redis_store.raw_get")
+    mock_redis_get = mocker.patch("app.celery.scheduled_tasks.redis_store.get")
     mock_redis_get.return_value = "foo"
 
-    mocker.patch("app.celery.scheduled_tasks.redis_store.raw_set")
+    mocker.patch("app.celery.scheduled_tasks.redis_store.set")
 
     data = {"to": None, "next": "/services"}
     admin_request.post(
@@ -581,10 +581,10 @@ def test_user_verify_email_code_fails_if_code_already_used(
 def test_send_user_2fa_code_sends_from_number_for_international_numbers(
     client, sample_user, mocker, sms_code_template
 ):
-    mock_redis_get = mocker.patch("app.user.rest.redis_store.raw_get")
+    mock_redis_get = mocker.patch("app.user.rest.redis_store.get")
     mock_redis_get.return_value = "foo"
 
-    mocker.patch("app.user.rest.redis_store.raw_set")
+    mocker.patch("app.user.rest.redis_store.set")
 
     sample_user.mobile_number = "+601117224412"
     auth_header = create_admin_authorization_header()

tests/notifications_utils/clients/redis/test_redis_client.py

@@ -1,5 +1,5 @@
 import uuid
-from unittest.mock import Mock, call
+from unittest.mock import Mock
 
 import pytest
 from freezegun import freeze_time
@@ -59,30 +59,6 @@ def failing_redis_client(mocked_redis_client, delete_mock):
     return mocked_redis_client
 
 
-def test_should_not_raise_exception_if_raise_set_to_false(
-    app, caplog, failing_redis_client, mocker
-):
-    mock_logger = mocker.patch("flask.Flask.logger")
-
-    assert failing_redis_client.get("get_key") is None
-    assert failing_redis_client.set("set_key", "set_value") is None
-    assert failing_redis_client.incr("incr_key") is None
-    assert failing_redis_client.exceeded_rate_limit("rate_limit_key", 100, 100) is False
-    assert failing_redis_client.delete("delete_key") is None
-    assert failing_redis_client.delete("a", "b", "c") is None
-    assert failing_redis_client.delete_by_pattern("pattern") == 0
-
-    assert mock_logger.mock_calls == [
-        call.exception("Redis error performing get on get_key"),
-        call.exception("Redis error performing set on set_key"),
-        call.exception("Redis error performing incr on incr_key"),
-        call.exception("Redis error performing rate-limit-pipeline on rate_limit_key"),
-        call.exception("Redis error performing delete on delete_key"),
-        call.exception("Redis error performing delete on a, b, c"),
-        call.exception("Redis error performing delete-by-pattern on pattern"),
-    ]
-
-
 def test_should_raise_exception_if_raise_set_to_true(
     app,
     failing_redis_client,