fix: use InvalidDocumentException instead of ValueError (#2376)

samtranslator/model/eventsources/push.py

@@ -19,7 +19,7 @@
 from samtranslator.model.cognito import CognitoUserPool
 from samtranslator.translator import logical_id_generator
 from samtranslator.translator.arn_generator import ArnGenerator
-from samtranslator.model.exceptions import InvalidEventException, InvalidResourceException
+from samtranslator.model.exceptions import InvalidEventException, InvalidResourceException, InvalidDocumentException
 from samtranslator.swagger.swagger import SwaggerEditor
 from samtranslator.open_api.open_api import OpenApiEditor
 from samtranslator.utils.py27hash_fix import Py27Dict, Py27UniStr
@@ -1119,7 +1119,7 @@ def _get_permission(self, resources_to_link, stage):
         if resources_to_link["explicit_api"].get("DefinitionBody"):
             try:
                 editor = OpenApiEditor(resources_to_link["explicit_api"].get("DefinitionBody"))
-            except ValueError as e:
+            except InvalidDocumentException as e:
                 api_logical_id = self.ApiId.get("Ref") if isinstance(self.ApiId, dict) else self.ApiId
                 raise InvalidResourceException(api_logical_id, e)
 

samtranslator/open_api/open_api.py

@@ -37,10 +37,10 @@ def __init__(self, doc):
         modifications on this copy.
 
         :param dict doc: OpenApi document as a dictionary
-        :raises ValueError: If the input OpenApi document does not meet the basic OpenApi requirements.
+        :raises InvalidDocumentException: If the input OpenApi document does not meet the basic OpenApi requirements.
         """
         if not OpenApiEditor.is_valid(doc):
-            raise ValueError(
+            raise InvalidDocumentException(
                 "Invalid OpenApi document. "
                 "Invalid values or missing keys for 'openapi' or 'paths' in 'DefinitionBody'."
             )
@@ -175,7 +175,7 @@ def add_path(self, path, method=None):
 
         :param string path: Path name
         :param string method: HTTP method
-        :raises ValueError: If the value of `path` in Swagger is not a dictionary
+        :raises InvalidDocumentException: If the value of `path` in Swagger is not a dictionary
         """
         method = self._normalize_method_name(method)
 

samtranslator/swagger/swagger.py

@@ -1,5 +1,4 @@
 import copy
-import json
 import re
 
 from samtranslator.model.intrinsics import ref, make_conditional, fnSub, is_intrinsic_no_value
@@ -45,11 +44,11 @@ def __init__(self, doc):
         modifications on this copy.
 
         :param dict doc: Swagger document as a dictionary
-        :raises ValueError: If the input Swagger document does not meet the basic Swagger requirements.
+        :raises InvalidDocumentException: If the input Swagger document does not meet the basic Swagger requirements.
         """
 
         if not SwaggerEditor.is_valid(doc):
-            raise ValueError("Invalid Swagger document")
+            raise InvalidDocumentException("Invalid Swagger document")
 
         self._doc = copy.deepcopy(doc)
         self.paths = self._doc["paths"]
@@ -187,7 +186,9 @@ def add_lambda_integration(
 
         method = self._normalize_method_name(method)
         if self.has_integration(path, method):
-            raise ValueError("Lambda integration already exists on Path={}, Method={}".format(path, method))
+            raise InvalidDocumentException(
+                "Lambda integration already exists on Path={}, Method={}".format(path, method)
+            )
 
         self.add_path(path, method)
 
@@ -251,7 +252,7 @@ def add_state_machine_integration(
 
         method = self._normalize_method_name(method)
         if self.has_integration(path, method):
-            raise ValueError("Integration already exists on Path={}, Method={}".format(path, method))
+            raise InvalidDocumentException("Integration already exists on Path={}, Method={}".format(path, method))
 
         self.add_path(path, method)
 
@@ -388,7 +389,7 @@ def add_cors(
         :param integer/dict max_age: Maximum duration to cache the CORS Preflight request. Value is set on
             Access-Control-Max-Age header. Value can also be an intrinsic function dict.
         :param bool/None allow_credentials: Flags whether request is allowed to contain credentials.
-        :raises ValueError: When values for one of the allowed_* variables is empty
+        :raises InvalidTemplateException: When values for one of the allowed_* variables is empty
         """
 
         for path_item in self.get_conditional_contents(self.paths.get(path)):
@@ -1022,13 +1023,13 @@ def _add_iam_resource_policy_for_method(self, policy_list, effect, resource_list
         """
         This method generates a policy statement to grant/deny specific IAM users access to the API method and
         appends it to the swagger under `x-amazon-apigateway-policy`
-        :raises ValueError: If the effect passed in does not match the allowed values.
+        :raises InvalidDocumentException: If the effect passed in does not match the allowed values.
         """
         if not policy_list:
             return
 
         if effect not in ["Allow", "Deny"]:
-            raise ValueError("Effect must be one of {}".format(["Allow", "Deny"]))
+            raise InvalidDocumentException("Effect must be one of {}".format(["Allow", "Deny"]))
 
         if not isinstance(policy_list, (dict, list)):
             raise InvalidDocumentException(
@@ -1081,7 +1082,7 @@ def _add_ip_resource_policy_for_method(self, ip_list, conditional, resource_list
         """
         This method generates a policy statement to grant/deny specific IP address ranges access to the API method and
         appends it to the swagger under `x-amazon-apigateway-policy`
-        :raises ValueError: If the conditional passed in does not match the allowed values.
+        :raises InvalidDocumentException: If the conditional passed in does not match the allowed values.
         """
         if not ip_list:
             return
@@ -1090,7 +1091,7 @@ def _add_ip_resource_policy_for_method(self, ip_list, conditional, resource_list
             ip_list = [ip_list]
 
         if conditional not in ["IpAddress", "NotIpAddress"]:
-            raise ValueError("Conditional must be one of {}".format(["IpAddress", "NotIpAddress"]))
+            raise InvalidDocumentException("Conditional must be one of {}".format(["IpAddress", "NotIpAddress"]))
 
         self.resource_policy["Version"] = "2012-10-17"
         allow_statement = Py27Dict()
@@ -1122,11 +1123,11 @@ def _add_vpc_resource_policy_for_method(self, endpoint_dict, conditional, resour
         """
         This method generates a policy statement to grant/deny specific VPC/VPCE access to the API method and
         appends it to the swagger under `x-amazon-apigateway-policy`
-        :raises ValueError: If the conditional passed in does not match the allowed values.
+        :raises InvalidDocumentException: If the conditional passed in does not match the allowed values.
         """
 
         if conditional not in ["StringNotEquals", "StringEquals"]:
-            raise ValueError("Conditional must be one of {}".format(["StringNotEquals", "StringEquals"]))
+            raise InvalidDocumentException("Conditional must be one of {}".format(["StringNotEquals", "StringEquals"]))
 
         condition = Py27Dict()
         string_endpoint_list = endpoint_dict.get("StringEndpointList")

tests/openapi/test_openapi.py

@@ -20,13 +20,13 @@ def test_must_raise_on_valid_swagger(self):
             "swagger": "2.0",  # "openapi": "2.1.0"
             "paths": {"/foo": {}, "/bar": {}},
         }  # missing openapi key word
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             OpenApiEditor(valid_swagger)
 
     def test_must_raise_on_invalid_openapi(self):
 
         invalid_openapi = {"paths": {}}  # Missing "openapi" keyword
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             OpenApiEditor(invalid_openapi)
 
     def test_must_succeed_on_valid_openapi(self):
@@ -40,13 +40,13 @@ def test_must_succeed_on_valid_openapi(self):
     def test_must_fail_on_invalid_openapi_version(self):
         invalid_openapi = {"openapi": "2.3.0", "paths": {"/foo": {}, "/bar": {}}}
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             OpenApiEditor(invalid_openapi)
 
     def test_must_fail_on_invalid_openapi_version_2(self):
         invalid_openapi = {"openapi": "3.1.1.1", "paths": {"/foo": {}, "/bar": {}}}
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             OpenApiEditor(invalid_openapi)
 
     def test_must_succeed_on_valid_openapi3(self):

tests/swagger/test_swagger.py

@@ -18,7 +18,7 @@ class TestSwaggerEditor_init(TestCase):
     def test_must_raise_on_invalid_swagger(self):
 
         invalid_swagger = {"paths": {}}  # Missing "Swagger" keyword
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             SwaggerEditor(invalid_swagger)
 
     def test_must_succeed_on_valid_swagger(self):
@@ -32,13 +32,13 @@ def test_must_succeed_on_valid_swagger(self):
     def test_must_fail_on_invalid_openapi_version(self):
         invalid_swagger = {"openapi": "2.3.0", "paths": {"/foo": {}, "/bar": {}}}
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             SwaggerEditor(invalid_swagger)
 
     def test_must_fail_on_invalid_openapi_version_2(self):
         invalid_swagger = {"openapi": "3.1.1.1", "paths": {"/foo": {}, "/bar": {}}}
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             SwaggerEditor(invalid_swagger)
 
     def test_must_succeed_on_valid_openapi3(self):
@@ -53,7 +53,7 @@ def test_must_succeed_on_valid_openapi3(self):
     def test_must_fail_with_bad_values_for_path(self, invalid_path_item):
         invalid_swagger = {"openapi": "3.1.1.1", "paths": {"/foo": {}, "/bad": invalid_path_item}}
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             SwaggerEditor(invalid_swagger)
 
 
@@ -261,7 +261,7 @@ def test_must_add_new_integration_to_existing_path(self):
 
     def test_must_raise_on_existing_integration(self):
 
-        with self.assertRaises(ValueError):
+        with self.assertRaises(InvalidDocumentException):
             self.editor.add_lambda_integration("/bar", "get", "integrationUri")
 
     def test_must_add_credentials_to_the_integration(self):