Merge pull request #113 from stankudrow/remove-strict-option-from-par…

…se-dsn-and-make-the-function-stricter

Remove the `strict` option from the `parse_dsn` function and make the function stricter on URL validation

CHANGELOG.md

@@ -4,8 +4,9 @@
 
 ### 0.2.5
 
+- Add more validation rules in the `parse_dsn` function. By @stankudrow in #113
 - Reconsider the API of the `Connection`, `Cursor` and `Pool` classes and deprecate outdated methods or properties. Define the DB-API v2.0 compliant exception hierarchy. Update project dependencies and metadata. By @stankudrow in #111.
-- Fix infinite iteration case when a cursor object is put in the `async for` loop. By @stankudrow in #112.
+- Fix infinite iteration case when a cursor object is put in the `async for` loop (the discussion #100 by @KuzenkovAG). By @stankudrow in #112.
 - Fix pool connection management (the discussion #108 by @DFilyushin) by @stankudrow in #109:
 
   - add the asynchronous context manager support to the `Pool` class with the pool "startup()" as `__aenter__` and "shutdown()" as `__aexit__` methods;

asynch/connection.py

@@ -53,14 +53,6 @@ def __init__(
         self._closed: Optional[bool] = None
         self._cursor_cls = cursor_cls
         self._connection_kwargs = kwargs
-        warn(
-            (
-                "The `echo` flag in the constructor is deprecated since the v0.2.5. "
-                "This flag is specifiable in the `cursor` method of the Connection class. "
-                "The `echo` parameter may be removed in the version 0.2.6 or later."
-            ),
-            DeprecationWarning,
-        )
         self._echo = echo
 
     async def __aenter__(self) -> "Connection":
@@ -175,13 +167,6 @@ def database(self) -> str:
 
     @property
     def echo(self) -> bool:
-        warn(
-            (
-                "The `echo` parameter should be specified in the `cursor` method."
-                "The property may be removed in the version 0.2.6 or later."
-            ),
-            DeprecationWarning,
-        )
         return self._echo
 
     async def close(self) -> None:
@@ -215,21 +200,13 @@ def cursor(self, cursor: Optional[Cursor] = None, *, echo: bool = False) -> Curs
         set to True even if the `self.echo` property returns False.
 
         :param cursor Optional[Cursor]: Cursor factory class
-        :param echo bool:
+        :param echo bool: to override the `Connection.echo` parametre for a cursor
 
         :return: the cursor object of a connection
         :rtype: Cursor
         """
 
         cursor_cls = cursor or self._cursor_cls
-        warn(
-            (
-                "When `echo` parameter is set to False (by default), "
-                "the deprecated `self.echo` property is in effect ."
-                "This behaviour may be removed in the version 0.2.6 or later."
-            ),
-            UserWarning,
-        )
         return cursor_cls(self, echo or self.echo)
 
     async def ping(self) -> None:
@@ -265,9 +242,6 @@ async def connect(
     When the connection is no longer needed,
     consider `await`ing the `conn.close()` method.
 
-    The `echo` parameter is deprecated since the version 0.2.5.
-    It may be removed in the version 0.2.6 or later.
-
     :param dsn str: DSN/connection string (if None -> constructed from default dsn parts)
     :param user str: user string ("default" by default)
     :param password str: password string ("" by default)

asynch/proto/utils/dsn.py

@@ -5,6 +5,8 @@
 from asynch.proto.models.enums import CompressionAlgorithms, Schemes
 from asynch.proto.utils.compat import asbool
 
+_SCHEME_SEPARATOR = "://"
+
 _COMPRESSION_ALGORITHMS: set[str] = {
     CompressionAlgorithms.lz4,
     CompressionAlgorithms.lz4hc,
@@ -18,7 +20,7 @@ class DSNError(Exception):
     pass
 
 
-def parse_dsn(dsn: str, *, strict: bool = False) -> dict[str, Any]:
+def parse_dsn(dsn: str) -> dict[str, Any]:
     """Return the client configuration from the given URL.
 
     The following URL schemes are supported:
@@ -30,18 +32,24 @@ def parse_dsn(dsn: str, *, strict: bool = False) -> dict[str, Any]:
     - clickhouses://[user:password]@localhost:9440/default
 
     :param dsn str: the DSN string
-    :param strict bool: enable the strict parsing mode
 
     :raises DSNError: when parsing fails under the strict mode
 
     :return: the dictionary of DSN string components
     :rtype: dict[str, Any]
     """
 
-    scheme, _, _ = dsn.partition("://")
-    if strict and (scheme not in _SUPPORTED_SCHEMES):
+    scheme, sep, rest = dsn.partition(_SCHEME_SEPARATOR)
+
+    if not sep:
+        msg = f"no valid scheme separator in the {dsn}"
+        raise DSNError(msg)
+    if scheme not in _SUPPORTED_SCHEMES:
         msg = f"the scheme {scheme!r} is not in {_SUPPORTED_SCHEMES}"
         raise DSNError(msg)
+    if not rest:
+        msg = f"nothing to parse after the scheme in the {dsn}"
+        raise DSNError(msg)
 
     settings = {}
     kwargs = {}

tests/test_pool.py

@@ -4,7 +4,7 @@
 import pytest
 
 from asynch.connection import Connection
-from asynch.pool import Pool, AsynchPoolError
+from asynch.pool import AsynchPoolError, Pool
 from asynch.proto import constants
 from asynch.proto.models.enums import PoolStatuses
 

tests/test_proto/utils/test_dsn.py

@@ -9,16 +9,14 @@
 
 
 @pytest.mark.parametrize(
-    ("dsn", "strict", "ctx", "answer"),
+    ("dsn", "ctx", "answer"),
     [
-        ("", False, does_not_raise(), {}),
-        ("some_scheme://", False, does_not_raise(), {"database": "some_scheme:/"}),
-        ("some_scheme://", True, pytest.raises(DSNError), None),
-        (f"{Schemes.clickhouse}://", True, does_not_raise(), {}),
-        (f"{Schemes.clickhouses}://", True, does_not_raise(), {"secure": True}),
+        ("", pytest.raises(DSNError), None),
+        ("some_scheme://", pytest.raises(DSNError), None),
+        (f"{Schemes.clickhouse}://", pytest.raises(DSNError), None),
+        (f"{Schemes.clickhouses}://", pytest.raises(DSNError), None),
         (
             f"{Schemes.clickhouse}://ch@lochost/",
-            False,
             does_not_raise(),
             {
                 "user": "ch",
@@ -27,7 +25,6 @@
         ),
         (
             f"{Schemes.clickhouse}://ch:pwd@lochost/",
-            False,
             does_not_raise(),
             {
                 "user": "ch",
@@ -37,7 +34,6 @@
         ),
         (
             f"{Schemes.clickhouse}://ch@lochost:4321/",
-            False,
             does_not_raise(),
             {
                 "user": "ch",
@@ -47,7 +43,6 @@
         ),
         (
             f"{Schemes.clickhouse}://ch:pwd@lochost:1234/db",
-            False,
             does_not_raise(),
             {
                 "user": "ch",
@@ -57,13 +52,28 @@
                 "database": "db",
             },
         ),
+        (
+            "lochost:1234/test",
+            pytest.raises(DSNError),
+            None,
+        ),
+        (
+            f"{Schemes.clickhouse}://lochost:1234/test",
+            does_not_raise(),
+            {"host": "lochost", "port": 1234, "database": "test"},
+        ),
+        (
+            f"{Schemes.clickhouse} :// lochost : 1234 / test",
+            pytest.raises(DSNError),
+            None,
+        ),
     ],
 )
 def test_dsn_basic_credentials(
-    dsn: str, strict: bool, ctx: Optional[ContextManager], answer: Optional[dict[str, Any]]
+    dsn: str, ctx: Optional[ContextManager], answer: Optional[dict[str, Any]]
 ) -> None:
     with ctx:
-        result = parse_dsn(dsn, strict=strict)
+        result = parse_dsn(dsn=dsn)
 
         assert result == answer
 
@@ -82,7 +92,7 @@ def test_dsn_basic_credentials(
             },
         ),
         (
-            "clickhouse://ch:pwd@loc:2938/ault",
+            f"{Schemes.clickhouse}://ch:pwd@loc:2938/ault",
             (
                 "verify=true"
                 "&secure=yes"
@@ -115,7 +125,7 @@ def test_dsn_with_query_fragments(
     answer: dict[str, Any],
 ) -> None:
     url = f"{dsn}?{query}"
-    config = parse_dsn(url, strict=True)
+    config = parse_dsn(dsn=url)
 
     for key, value in answer.items():
         if value is None: