refactor: unique function name validation (#236)

hallvictoria · Victoria Hall · web-flow · commit 0617cd7bf1de · 2024-08-07T14:37:57.000-05:00
* set functions_bindings to None

* refactor name tracker to FunctionRegister

* lint

* renamed validate_functions

* feedback

* missed test fix

* missed test fix

* lint

* fix tests

* replace kafka test value

* remove kafka test value

---------

Co-authored-by: Victoria Hall &lt;victoria.hall@microsoft.com&gt;
diff --git a/azure/functions/decorators/function_app.py b/azure/functions/decorators/function_app.py
@@ -206,7 +206,6 @@ def __str__(self):
 
 
 class FunctionBuilder(object):
-    function_bindings: dict = {}
 
     def __init__(self, func, function_script_file):
         self._function = Function(func, function_script_file)
@@ -272,16 +271,6 @@ def _validate_function(self,
                         parse_singular_param_to_enum(auth_level, AuthLevel))
             self._function._is_http_function = True
 
-        # This dict contains the function name and its bindings for all
-        # functions in an app. If a previous function has the same name,
-        # indexing will fail here.
-        if self.function_bindings.get(function_name, None):
-            raise ValueError(
-                f"Function {function_name} does not have a unique"
-                f" function name. Please change @app.function_name() or"
-                f" the function method name to be unique.")
-        self.function_bindings[function_name] = bindings
-
     def build(self, auth_level: Optional[AuthLevel] = None) -> Function:
         """
         Validates and builds the function object.
@@ -3592,6 +3581,7 @@ def __init__(self, auth_level: Union[AuthLevel, str], *args, **kwargs):
         DecoratorApi.__init__(self, *args, **kwargs)
         HttpFunctionsAuthLevelMixin.__init__(self, auth_level, *args, **kwargs)
         self._require_auth_level: Optional[bool] = None
+        self.functions_bindings: Optional[Dict[Any, Any]] = None
 
     def get_functions(self) -> List[Function]:
         """Get the function objects in the function app.
@@ -3613,8 +3603,28 @@ def get_functions(self) -> List[Function]:
                 '-bindings-http-webhook-trigger?tabs=in-process'
                 '%2Cfunctionsv2&pivots=programming-language-python#http-auth')
 
+        self.validate_function_names(functions=functions)
+
         return functions
 
+    def validate_function_names(self, functions: List[Function]):
+        """The functions_bindings dict contains the function name and
+        its bindings for all functions in an app. If a previous function
+        has the same name, indexing will fail here.
+        """
+        if not self.functions_bindings:
+            self.functions_bindings = {}
+        for function in functions:
+            function_name = function.get_function_name()
+            if function_name in self.functions_bindings:
+                raise ValueError(
+                    f"Function {function_name} does not have a unique"
+                    f" function name. Please change @app.function_name() or"
+                    f" the function method name to be unique.")
+            # The value of the key doesn't matter. We're using a dict for
+            # faster lookup times.
+            self.functions_bindings[function_name] = True
+
     def register_functions(self, function_container: DecoratorApi) -> None:
         """Register a list of functions in the function app.
 
diff --git a/tests/decorators/test_function_app.py b/tests/decorators/test_function_app.py
@@ -20,7 +20,6 @@
 )
 from azure.functions.decorators.http import HttpTrigger, HttpOutput, \
     HttpMethod
-from azure.functions.decorators.timer import TimerTrigger
 from azure.functions.decorators.retry_policy import RetryPolicy
 from test_core import DummyTrigger
 from tests.utils.testutils import assert_json
@@ -291,10 +290,6 @@ def test_unique_method_names2(name: str):
                          "test_unique_method_names")
         self.assertEqual(functions[1].get_function_name(),
                          "test_unique_method_names2")
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_unique_method_names")[0], TimerTrigger)
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_unique_method_names2")[0], TimerTrigger)
 
     def test_unique_function_names(self):
         app = FunctionApp()
@@ -316,10 +311,6 @@ def test_unique_function_names2(name: str):
                          "test_unique_function_names")
         self.assertEqual(functions[1].get_function_name(),
                          "test_unique_function_names2")
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_unique_function_names")[0], TimerTrigger)
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_unique_function_names2")[0], TimerTrigger)
 
     def test_same_method_names(self):
         app = FunctionApp()
@@ -425,10 +416,6 @@ def test_blueprint_unique_method_names2(name: str):
                          "test_blueprint_unique_method_names")
         self.assertEqual(functions[1].get_function_name(),
                          "test_blueprint_unique_method_names2")
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_blueprint_unique_method_names")[0], TimerTrigger)
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_blueprint_unique_method_names2")[0], TimerTrigger)
 
     def test_blueprint_unique_function_names(self):
         app = FunctionApp()
@@ -454,10 +441,6 @@ def test_blueprint_unique_function_names2(name: str):
                          "test_blueprint_unique_function_names")
         self.assertEqual(functions[1].get_function_name(),
                          "test_blueprint_unique_function_names2")
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_blueprint_unique_function_names")[0], TimerTrigger)
-        self.assertIsInstance(app._function_builders[0].function_bindings.get(
-            "test_blueprint_unique_function_names2")[0], TimerTrigger)
 
     def test_blueprint_same_method_names(self):
         app = FunctionApp()
@@ -1009,3 +992,43 @@ def _test_http_external_app(self, app, is_async, function_name):
                     "type": HTTP_OUTPUT
                 }
             ]})
+
+
+class TestFunctionRegister(unittest.TestCase):
+    def test_validate_empty_dict(self):
+        def dummy():
+            return "dummy"
+
+        test_func = Function(dummy, "dummy.py")
+        fr = FunctionRegister(auth_level="ANONYMOUS")
+        fr.validate_function_names(functions=[test_func])
+
+    def test_validate_unique_names(self):
+        def dummy():
+            return "dummy"
+
+        def dummy2():
+            return "dummy"
+
+        test_func = Function(dummy, "dummy.py")
+        test_func2 = Function(dummy2, "dummy.py")
+
+        fr = FunctionRegister(auth_level="ANONYMOUS")
+        fr.validate_function_names(
+            functions=[test_func, test_func2])
+
+    def test_validate_non_unique_names(self):
+        def dummy():
+            return "dummy"
+
+        test_func = Function(dummy, "dummy.py")
+        test_func2 = Function(dummy, "dummy.py")
+
+        fr = FunctionRegister(auth_level="ANONYMOUS")
+        with self.assertRaises(ValueError) as err:
+            fr.validate_function_names(functions=[test_func, test_func2])
+            self.assertEqual(err.exception.args[0],
+                             "Function dummy does not have"
+                             " a unique function name."
+                             " Please change @app.function_name()"
+                             " or the function method name to be unique.")
diff --git a/tests/decorators/test_kafka.py b/tests/decorators/test_kafka.py
@@ -24,7 +24,7 @@ def test_kafka_trigger_valid_creation(self):
                                ssl_certificate_location="scl",
                                ssl_key_password="ssl_key_password",
                                schema_registry_url="srurl",
-                               schema_registry_username="sruser",
+                               schema_registry_username="",
                                schema_registry_password="srp",
                                authentication_mode=BrokerAuthenticationMode.PLAIN,  # noqa: E501
                                data_type=DataType.UNDEFINED,
@@ -46,7 +46,7 @@ def test_kafka_trigger_valid_creation(self):
                           "protocol": BrokerProtocol.NOTSET,
                           "schemaRegistryPassword": "srp",
                           "schemaRegistryUrl": "srurl",
-                          "schemaRegistryUsername": "sruser",
+                          "schemaRegistryUsername": "",
                           "sslCaLocation": "ssl_ca_location",
                           "sslCertificateLocation": "scl",
                           "sslKeyLocation": "ssl_key_location",
@@ -68,7 +68,7 @@ def test_kafka_output_valid_creation(self):
                              ssl_certificate_location="scl",
                              ssl_key_password="ssl_key_password",
                              schema_registry_url="schema_registry_url",
-                             schema_registry_username="sru",
+                             schema_registry_username="",
                              schema_registry_password="srp",
                              max_retries=10,
                              data_type=DataType.UNDEFINED,
@@ -94,7 +94,7 @@ def test_kafka_output_valid_creation(self):
                           'requestTimeoutMs': 5000,
                           'schemaRegistryPassword': 'srp',
                           'schemaRegistryUrl': 'schema_registry_url',
-                          'schemaRegistryUsername': 'sru',
+                          'schemaRegistryUsername': '',
                           'sslCaLocation': 'ssl_ca_location',
                           'sslCertificateLocation': 'scl',
                           'sslKeyLocation': 'ssl_key_location',
