VED-79: modularize error, fix issues and increase coverage

Author: Akol125

mns_subscription/models/errors.py

@@ -115,6 +115,25 @@ def to_operation_outcome(self) -> dict:
         )
 
 
+@dataclass
+class BadRequestError(RuntimeError):
+    """Use when payload is missing required parameters"""
+
+    response: dict | str
+    message: str
+
+    def __str__(self):
+        return f"{self.message}\n{self.response}"
+
+    def to_operation_outcome(self) -> dict:
+        return create_operation_outcome(
+            resource_id=str(uuid.uuid4()),
+            severity=Severity.error,
+            code=Code.server_error,
+            diagnostics=self.__str__(),
+        )
+
+
 @dataclass
 class ServerError(RuntimeError):
     """Use when there is a server error"""

mns_subscription/src/mns_service.py

@@ -9,7 +9,9 @@
     ResourceFoundError,
     UnauthorizedError,
     ServerError,
-    TokenValidationError
+    BadRequestError,
+    TokenValidationError,
+    ConflictError
 )
 
 SQS_ARN = os.getenv("SQS_ARN")
@@ -54,24 +56,8 @@ def subscribe_notification(self) -> dict | None:
 
         if response.status_code in (200, 201):
             return response.json()
-        elif response.status_code == 409:
-            msg = "SQS Queue Already Subscribed, can't re-subscribe"
-            raise UnhandledResponseError(response=response.json(), message=msg)
-        elif response.status_code == 401:
-            msg = "SQS Queue Already Subscribed, can't re-subscribe"
-            raise TokenValidationError(response=response.json(), message=msg)
-        elif response.status_code == 400:
-            msg = "Resource Type provided for this is not correct"
-            raise ResourceFoundError(response=response.json(), message=msg)
-        elif response.status_code == 403:
-            msg = "You don't have the right permissions for this request"
-            raise UnauthorizedError(response=response.json(), message=msg)
-        elif response.status_code == 500:
-            msg = "Internal Server Error"
-            raise ServerError(response=response.json(), message=msg)
         else:
-            msg = f"Unhandled error: {response.status_code} - {response.text}"
-            raise UnhandledResponseError(response=response.json(), message=msg)
+            MnsService.handle_response(response)
 
     def get_subscription(self) -> dict | None:
         response = requests.get(MNS_URL, headers=self.request_headers)
@@ -80,30 +66,16 @@ def get_subscription(self) -> dict | None:
 
         if response.status_code == 200:
             bundle = response.json()
-            # Assume a FHIR Bundle with 'entry' list
             for entry in bundle.get("entry", []):
                 resource = entry.get("resource", entry)
                 print(f"get resource sub: {resource}")
                 logging.debug(f"get resource sub: {resource}")
                 channel = resource.get("channel", {})
                 if channel.get("endpoint") == SQS_ARN:
-                    return resource  # Found a matching subscription
-            return None  # No subscription for this SQS ARN
-        elif response.status_code == 401:
-            msg = "Token validation failed for the request"
-            raise TokenValidationError(response=response.json(), message=msg)
-        elif response.status_code == 400:
-            msg = "Bad request: Resource type or parameters incorrect"
-            raise ResourceFoundError(response=response.json(), message=msg)
-        elif response.status_code == 403:
-            msg = "You don't have the right permissions for this request"
-            raise UnauthorizedError(response=response.json(), message=msg)
-        elif response.status_code == 500:
-            msg = "Internal Server Error"
-            raise ServerError(response=response.json(), message=msg)
+                    return resource
+            return None
         else:
-            msg = f"Unhandled error: {response.status_code} - {response.text}"
-            raise UnhandledResponseError(response=response.json(), message=msg)
+            MnsService.handle_response(response)
 
     def check_subscription(self) -> dict:
         """
@@ -130,20 +102,12 @@ def delete_subscription(self, subscription_id: str) -> bool:
         if response.status_code in (200, 204):
             logging.info(f"Deleted subscription {subscription_id}")
             return True
-        elif response.status_code == 401:
-            raise TokenValidationError(response=response.json(), message="Token validation failed for the request")
-        elif response.status_code == 404:
-            raise ResourceFoundError(response=response.json(), message=f"Subscription {subscription_id} not found")
-        elif response.status_code == 403:
-            raise UnauthorizedError(response=response.json(), message="No permission to delete subscription")
-        elif response.status_code == 500:
-            raise ServerError(response=response.json(), message="Internal Server Error")
         else:
-            raise UnhandledResponseError(response=response.json(), message=f"Unhandled error: {response.status_code}")
+            MnsService.handle_response(response)
 
     def check_delete_subcription(self):
         try:
-            resource = self.get_subscription()  # Get the full resource dict
+            resource = self.get_subscription()
             if not resource:
                 return "No matching subscription found to delete."
 
@@ -154,5 +118,20 @@ def check_delete_subcription(self):
             self.delete_subscription(subscription_id)
             return "Subscription successfully deleted"
         except Exception as e:
-            # Optionally log the exception here
             return f"Error deleting subscription: {str(e)}"
+
+    @staticmethod
+    def handle_response(response):
+        error_mapping = {
+            401: (TokenValidationError, "Token validation failed for the request"),
+            400: (BadRequestError, "Bad request: Resource type or parameters incorrect"),
+            403: (UnauthorizedError, "You don't have the right permissions for this request"),
+            500: (ServerError, "Internal Server Error"),
+            404: (ResourceFoundError, "Subscription or Resource not found"),
+            409: (ConflictError, "SQS Queue Already Subscribed, can't re-subscribe")
+        }
+        exception_class, error_message = error_mapping.get(
+            response.status_code,
+            (UnhandledResponseError, f"Unhandled error: {response.status_code}"))
+
+        raise exception_class(response=response.json(), message=error_message)

mns_subscription/tests/test_mns_service.py

@@ -7,6 +7,7 @@
     ServerError,
     UnhandledResponseError,
     TokenValidationError,
+    BadRequestError,
     UnauthorizedError,
     ResourceFoundError)
 
@@ -31,7 +32,7 @@ def test_successful_subscription(self, mock_get, mock_post):
         # Arrange GET to return no subscription found
         mock_get_response = MagicMock()
         mock_get_response.status_code = 200
-        mock_get_response.json.return_value = {"entry": []}  # No entries!
+        mock_get_response.json.return_value = {"entry": []}
         mock_get.return_value = mock_get_response
 
         # Arrange
@@ -59,10 +60,9 @@ def test_not_found_subscription(self, mock_post):
 
         service = MnsService(self.authenticator)
 
-        with self.assertRaises(UnhandledResponseError) as context:
+        with self.assertRaises(ResourceFoundError) as context:
             service.subscribe_notification()
-        self.assertIn("404", str(context.exception))
-        self.assertIn("Unhandled error", str(context.exception))
+        self.assertIn("Subscription or Resource not found", str(context.exception))
 
     @patch("mns_service.requests.post")
     def test_unhandled_error(self, mock_post):
@@ -105,7 +105,7 @@ def test_get_subscription_no_match(self, mock_get):
         """Should return None when no subscription matches."""
         mock_response = MagicMock()
         mock_response.status_code = 200
-        mock_response.json.return_value = {"entry": []}  # No matches
+        mock_response.json.return_value = {"entry": []}
         mock_get.return_value = mock_response
 
         service = MnsService(self.authenticator)
@@ -124,8 +124,6 @@ def test_get_subscription_401(self, mock_get):
         with self.assertRaises(TokenValidationError):
             service.get_subscription()
 
-    # Similarly, you can add tests for 400, 403, 500, etc.
-
     @patch("mns_service.requests.post")
     @patch("mns_service.requests.get")
     def test_check_subscription_creates_if_not_found(self, mock_get, mock_post):
@@ -150,7 +148,6 @@ def test_check_subscription_creates_if_not_found(self, mock_get, mock_post):
 
     @patch("mns_service.requests.delete")
     def test_delete_subscription_success(self, mock_delete):
-        # Test for both 200 and 204 (use a loop)
         for code in (200, 204):
             mock_response = MagicMock()
             mock_response.status_code = code
@@ -258,6 +255,35 @@ def test_check_delete_subscription_raises(self, mock_get_subscription, mock_dele
         result = service.check_delete_subcription()
         self.assertTrue(result.startswith("Error deleting subscription: Error!"))
 
+    def mock_response(self, status_code, json_data=None):
+        mock_resp = MagicMock()
+        mock_resp.status_code = status_code
+        mock_resp.json.return_value = json_data or {"resource": "mock"}
+        return mock_resp
+
+    def test_404_resource_found_error(self):
+        resp = self.mock_response(404, {"resource": "Not found"})
+        with self.assertRaises(ResourceFoundError) as context:
+            MnsService.handle_response(resp)
+        self.assertIn("Subscription or Resource not found", str(context.exception))
+        self.assertEqual(context.exception.message, "Subscription or Resource not found")
+        self.assertEqual(context.exception.response, {"resource": "Not found"})
+
+    def test_400_bad_request_error(self):
+        resp = self.mock_response(400, {"resource": "Invalid"})
+        with self.assertRaises(BadRequestError) as context:
+            MnsService.handle_response(resp)
+        self.assertIn("Bad request: Resource type or parameters incorrect", str(context.exception))
+        self.assertEqual(context.exception.message, "Bad request: Resource type or parameters incorrect")
+        self.assertEqual(context.exception.response, {"resource": "Invalid"})
+
+    def test_unhandled_status_code(self):
+        resp = self.mock_response(418, {"resource": 1234})
+        with self.assertRaises(UnhandledResponseError) as context:
+            MnsService.handle_response(resp)
+        self.assertIn("Unhandled error: 418", str(context.exception))
+        self.assertEqual(context.exception.response, {"resource": 1234})
+
 
 if __name__ == "__main__":
     unittest.main()