refactor: config properties retrieval and validation (#1053)

karenc-bq · sophia-bq · commit 9e9ac20280c5 · 2025-12-15T11:36:20.000-08:00
diff --git a/aws_advanced_python_wrapper/simple_read_write_splitting_plugin.py b/aws_advanced_python_wrapper/simple_read_write_splitting_plugin.py
@@ -15,7 +15,7 @@
 from __future__ import annotations
 
 from time import perf_counter_ns, sleep
-from typing import TYPE_CHECKING, Callable, Optional
+from typing import TYPE_CHECKING, Callable, Optional, TypeVar, Type
 
 from aws_advanced_python_wrapper.host_availability import HostAvailability
 from aws_advanced_python_wrapper.read_write_splitting_plugin import (
@@ -28,7 +28,7 @@
     from aws_advanced_python_wrapper.host_list_provider import HostListProviderService
     from aws_advanced_python_wrapper.pep249 import Connection
     from aws_advanced_python_wrapper.plugin_service import PluginService
-    from aws_advanced_python_wrapper.utils.properties import Properties
+    from aws_advanced_python_wrapper.utils.properties import Properties, WrapperProperty
 
 from aws_advanced_python_wrapper.errors import AwsWrapperError
 from aws_advanced_python_wrapper.hostinfo import HostInfo, HostRole
@@ -41,53 +41,24 @@ class EndpointBasedConnectionHandler(ConnectionHandler):
     """Endpoint based implementation of connection handling logic."""
 
     def __init__(self, plugin_service: PluginService, props: Properties):
-        srw_read_endpoint = WrapperProperties.SRW_READ_ENDPOINT.get(props)
-        if srw_read_endpoint is None:
-            raise AwsWrapperError(
-                Messages.get_formatted(
-                    "SimpleReadWriteSplittingPlugin.MissingRequiredConfigParameter",
-                    WrapperProperties.SRW_READ_ENDPOINT.name,
-                )
-            )
-        self._read_endpoint: str = srw_read_endpoint
-
-        srw_write_endpoint = WrapperProperties.SRW_WRITE_ENDPOINT.get(props)
-        if srw_write_endpoint is None:
-            raise AwsWrapperError(
-                Messages.get_formatted(
-                    "SimpleReadWriteSplittingPlugin.MissingRequiredConfigParameter",
-                    WrapperProperties.SRW_WRITE_ENDPOINT.name,
-                )
-            )
-        self._write_endpoint: str = srw_write_endpoint
+        self._read_endpoint: str = EndpointBasedConnectionHandler._verify_parameter(
+            WrapperProperties.SRW_READ_ENDPOINT, props, str, required=True
+        )
+        self._write_endpoint: str = EndpointBasedConnectionHandler._verify_parameter(
+            WrapperProperties.SRW_WRITE_ENDPOINT, props, str, required=True
+        )
 
-        self._verify_new_connections: bool = (
-            WrapperProperties.SRW_VERIFY_NEW_CONNECTIONS.get_bool(props)
+        self._verify_new_connections: bool = EndpointBasedConnectionHandler._verify_parameter(
+            WrapperProperties.SRW_VERIFY_NEW_CONNECTIONS, props, bool
         )
-        if self._verify_new_connections is True:
-            srw_connect_retry_timeout_ms: int = (
-                WrapperProperties.SRW_CONNECT_RETRY_TIMEOUT_MS.get_int(props)
-            )
-            if srw_connect_retry_timeout_ms <= 0:
-                raise ValueError(
-                    Messages.get_formatted(
-                        "SimpleReadWriteSplittingPlugin.IncorrectConfiguration",
-                        WrapperProperties.SRW_CONNECT_RETRY_TIMEOUT_MS.name,
-                    )
-                )
-            self._connect_retry_timeout_ms: int = srw_connect_retry_timeout_ms
 
-            srw_connect_retry_interval_ms: int = (
-                WrapperProperties.SRW_CONNECT_RETRY_INTERVAL_MS.get_int(props)
+        if self._verify_new_connections:
+            self._connect_retry_timeout_ms: int = EndpointBasedConnectionHandler._verify_parameter(
+                WrapperProperties.SRW_CONNECT_RETRY_TIMEOUT_MS, props, int, lambda x: x > 0
+            )
+            self._connect_retry_interval_ms: int = EndpointBasedConnectionHandler._verify_parameter(
+                WrapperProperties.SRW_CONNECT_RETRY_INTERVAL_MS, props, int, lambda x: x > 0
             )
-            if srw_connect_retry_interval_ms <= 0:
-                raise ValueError(
-                    Messages.get_formatted(
-                        "SimpleReadWriteSplittingPlugin.IncorrectConfiguration",
-                        WrapperProperties.SRW_CONNECT_RETRY_INTERVAL_MS.name,
-                    )
-                )
-            self._connect_retry_interval_ms: int = srw_connect_retry_interval_ms
 
             self._verify_opened_connection_type: Optional[HostRole] = (
                 EndpointBasedConnectionHandler._parse_connection_type(
@@ -305,6 +276,29 @@ def _create_host_info(self, endpoint, role: HostRole) -> HostInfo:
             host=host, port=port, role=role, availability=HostAvailability.AVAILABLE
         )
 
+    T = TypeVar('T')
+
+    @staticmethod
+    def _verify_parameter(prop: WrapperProperty, props: Properties, expected_type: Type[T], validator=None, required=False):
+        value = prop.get_type(props, expected_type)
+        if required:
+            if value is None:
+                raise AwsWrapperError(
+                    Messages.get_formatted(
+                        "SimpleReadWriteSplittingPlugin.MissingRequiredConfigParameter",
+                        prop.name,
+                    )
+                )
+
+        if validator and not validator(value):
+            raise ValueError(
+                Messages.get_formatted(
+                    "SimpleReadWriteSplittingPlugin.IncorrectConfiguration",
+                    prop.name,
+                )
+            )
+        return value
+
     def _delay(self):
         sleep(self._connect_retry_interval_ms / 1000)
 
diff --git a/aws_advanced_python_wrapper/utils/properties.py b/aws_advanced_python_wrapper/utils/properties.py
@@ -11,8 +11,9 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
+
 import copy
-from typing import Any, Dict, Optional
+from typing import Any, Dict, Optional, TypeVar, Type
 from urllib.parse import unquote
 
 from aws_advanced_python_wrapper.errors import AwsWrapperError
@@ -25,6 +26,9 @@ def put_if_absent(self, key: str, value: Any):
             self[key] = value
 
 
+T = TypeVar('T')
+
+
 class WrapperProperty:
     def __init__(
         self, name: str, description: str, default_value: Optional[Any] = None
@@ -34,41 +38,43 @@ def __init__(
         self.description = description
 
     def __str__(self):
-        return f"WrapperProperty(name={self.name}, default_value={self.default_value}"
+        return f"WrapperProperty(name={self.name}, default_value={self.default_value})"
 
     def get(self, props: Properties) -> Optional[str]:
         if self.default_value:
             return props.get(self.name, self.default_value)
         return props.get(self.name)
 
+    def get_type(self, props: Properties, type_class: Type[T]) -> T:
+        value = props.get(self.name, self.default_value) if self.default_value else props.get(self.name)
+        if value is None:
+            if type_class == int:
+                return -1  # type: ignore
+            elif type_class == float:
+                return -1.0  # type: ignore
+            elif type_class == bool:
+                return False  # type: ignore
+            else:
+                return None  # type: ignore
+        if type_class == bool:
+            if isinstance(value, bool):
+                return value  # type: ignore
+            return value.lower() == "true" if isinstance(value, str) else bool(value)  # type: ignore
+        return type_class(value)  # type: ignore
+
     def get_or_default(self, props: Properties) -> str:
         if not self.default_value:
             raise ValueError(f"No default value found for property {self}")
         return props.get(self.name, self.default_value)
 
     def get_int(self, props: Properties) -> int:
-        if self.default_value:
-            return int(props.get(self.name, self.default_value))
-
-        val = props.get(self.name)
-        return int(val) if val else -1
+        return self.get_type(props, int)
 
     def get_float(self, props: Properties) -> float:
-        if self.default_value:
-            return float(props.get(self.name, self.default_value))
-
-        val = props.get(self.name)
-        return float(val) if val else -1
+        return self.get_type(props, float)
 
     def get_bool(self, props: Properties) -> bool:
-        if not self.default_value:
-            value = props.get(self.name)
-        else:
-            value = props.get(self.name, self.default_value)
-        if isinstance(value, bool):
-            return value
-        else:
-            return value is not None and value.lower() == "true"
+        return self.get_type(props, bool)
 
     def set(self, props: Properties, value: Any):
         props[self.name] = value
diff --git a/tests/unit/test_properties_utils.py b/tests/unit/test_properties_utils.py
@@ -16,7 +16,8 @@
 
 from aws_advanced_python_wrapper.errors import AwsWrapperError
 from aws_advanced_python_wrapper.utils.properties import (Properties,
-                                                          PropertiesUtils)
+                                                          PropertiesUtils,
+                                                          WrapperProperty)
 
 
 @pytest.mark.parametrize(
@@ -93,3 +94,76 @@ def test_create_monitoring_properties(expected, test_props):
     props_copy = test_props.copy()
     props_copy = PropertiesUtils.create_monitoring_properties(props_copy)
     assert expected == props_copy
+
+
+@pytest.mark.parametrize("expected, props, type_class", [
+    # Int type tests
+    (123, Properties({"test_prop": "123"}), int),
+    (-1, Properties(), int),
+    (456, Properties({"test_prop": 456}), int),
+
+    # Float type tests
+    (12.5, Properties({"test_prop": "12.5"}), float),
+    (-1.0, Properties(), float),
+    (3.14, Properties({"test_prop": 3.14}), float),
+
+    # Bool type tests
+    (True, Properties({"test_prop": "true"}), bool),
+    (True, Properties({"test_prop": "TRUE"}), bool),
+    (False, Properties({"test_prop": "false"}), bool),
+    (True, Properties({"test_prop": True}), bool),
+    (False, Properties({"test_prop": False}), bool),
+    (False, Properties(), bool),
+
+    # String type tests
+    ("test_value", Properties({"test_prop": "test_value"}), str),
+    (None, Properties(), str),
+    ("", Properties({"test_prop": ""}), str),
+])
+def test_get_type(expected, props, type_class):
+    wrapper_prop = WrapperProperty("test_prop", "Test property")
+    result = wrapper_prop.get_type(props, type_class)
+    assert result == expected
+
+
+def test_get_type_with_default():
+    wrapper_prop = WrapperProperty("test_prop", "Test property", "default_value")
+    props = Properties()
+    result = wrapper_prop.get_type(props, str)
+    assert result == "default_value"
+
+
+@pytest.mark.parametrize("expected, props", [
+    (123, Properties({"test_prop": "123"})),
+    (-1, Properties()),
+    (456, Properties({"test_prop": 456})),
+])
+def test_get_int(expected, props):
+    wrapper_prop = WrapperProperty("test_prop", "Test property")
+    result = wrapper_prop.get_int(props)
+    assert result == expected
+
+
+@pytest.mark.parametrize("expected, props", [
+    (12.5, Properties({"test_prop": "12.5"})),
+    (-1.0, Properties()),
+    (3.14, Properties({"test_prop": 3.14})),
+])
+def test_get_float(expected, props):
+    wrapper_prop = WrapperProperty("test_prop", "Test property")
+    result = wrapper_prop.get_float(props)
+    assert result == expected
+
+
+@pytest.mark.parametrize("expected, props", [
+    (True, Properties({"test_prop": "true"})),
+    (True, Properties({"test_prop": "TRUE"})),
+    (False, Properties({"test_prop": "false"})),
+    (True, Properties({"test_prop": True})),
+    (False, Properties({"test_prop": False})),
+    (False, Properties()),
+])
+def test_get_bool(expected, props):
+    wrapper_prop = WrapperProperty("test_prop", "Test property")
+    result = wrapper_prop.get_bool(props)
+    assert result == expected
