aminghadersohi commented on code in PR #44718:
URL: https://github.com/apache/superset/pull/44718#discussion_r4129992263


##########
tests/unit_tests/db_engine_specs/test_doris.py:
##########
@@ -280,3 +280,214 @@ def test_get_catalog_names(
 
     # Verify the returned catalog names
     assert catalogs == expected_result
+
+
[email protected](
+    "native_type,generic_type",
+    [
+        ("variant", GenericDataType.STRING),
+        ("ipv4", GenericDataType.STRING),
+        ("ipv6", GenericDataType.STRING),
+        # MySQL protocol type names reported for SQL Lab result columns
+        ("NEWDECIMAL", GenericDataType.NUMERIC),
+        ("TINY", GenericDataType.NUMERIC),
+        ("SHORT", GenericDataType.NUMERIC),
+        ("BLOB", GenericDataType.STRING),
+    ],
+)
+def test_get_column_spec_extra_types(
+    native_type: str, generic_type: GenericDataType
+) -> None:
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    spec = DorisEngineSpec.get_column_spec(native_type)
+    assert spec is not None
+    assert spec.generic_type == generic_type
+
+
+def test_quarter_time_grain_avoids_interval_quarter() -> None:
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    expression = DorisEngineSpec.get_time_grain_expressions()["P3M"]
+    assert expression == (
+        "MAKEDATE(YEAR({col}), 1) + INTERVAL (QUARTER({col}) - 1) * 3 MONTH"
+    )
+    assert "INTERVAL 1 QUARTER" not in expression
+
+
[email protected](
+    "message,error_type",
+    [
+        (
+            "(2002, \"Can't connect to server on '127.0.0.1' (115)\")",
+            "CONNECTION_HOST_DOWN_ERROR",
+        ),
+        (
+            "(2003, \"Can't connect to MySQL server on 'db' (111)\")",
+            "CONNECTION_HOST_DOWN_ERROR",
+        ),
+        (
+            "(2005, \"Unknown server host 'no-such-host.invalid' (-2)\")",
+            "CONNECTION_INVALID_HOSTNAME_ERROR",
+        ),
+        (
+            "(1105, \"errCode = 2, detailMessage = \\nmismatched input 'SELEC' 
"
+            "expecting {<EOF>, ';'}\")",
+            "SYNTAX_ERROR",
+        ),
+        (
+            "(1105, 'errCode = 2, detailMessage = Table [missing] does not 
exist "
+            "in database [db].(line 1, pos 14)')",
+            "TABLE_DOES_NOT_EXIST_ERROR",
+        ),
+        (
+            "(1105, 'errCode = 2, detailMessage = Database [nodb] does not 
exist."
+            "(line 1, pos 14)')",
+            "SCHEMA_DOES_NOT_EXIST_ERROR",
+        ),
+        (
+            "(1105, \"errCode = 2, detailMessage = Unknown column 'nope' in "
+            "'table list' in PROJECT clause(line 1, pos 7)\")",
+            "COLUMN_DOES_NOT_EXIST_ERROR",
+        ),
+        (
+            "(1045, \"Access denied for user '[email protected]' (using password: 
YES)\")",
+            "CONNECTION_ACCESS_DENIED_ERROR",
+        ),
+        (
+            "(1049, \"errCode = 2, detailMessage = Unknown database 'nodb'\")",
+            "CONNECTION_UNKNOWN_DATABASE_ERROR",
+        ),
+    ],
+)
+def test_extract_errors(message: str, error_type: str) -> None:
+    """Driver errors map to the corresponding Superset error types."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    errors = DorisEngineSpec.extract_errors(Exception(message))
+    assert errors[0].error_type.name == error_type
+
+
+def test_build_sqlalchemy_uri() -> None:
+    """The parameters form builds a valid Doris URI with optional verified 
TLS."""
+    from superset.db_engine_specs.base import BasicParametersType
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    parameters: BasicParametersType = {
+        "username": "user",
+        "password": "p@ss",
+        "host": "doris.example.com",
+        "port": 9030,
+        "database": "internal.db",
+        "query": {},
+    }
+    encrypted = make_url(
+        DorisEngineSpec.build_sqlalchemy_uri({**parameters, "encryption": 
True})
+    )
+    assert encrypted.drivername == "doris"
+    assert dict(encrypted.query) == {"ssl_mode": "VERIFY_CA"}
+    assert encrypted.password == "p@ss"  # noqa: S105
+
+    plain = make_url(
+        DorisEngineSpec.build_sqlalchemy_uri({**parameters, "encryption": 
False})
+    )
+    assert plain.drivername == "doris"
+    assert dict(plain.query) == {}
+
+    round_trip = DorisEngineSpec.get_parameters_from_uri(
+        encrypted.render_as_string(hide_password=False)
+    )
+    assert round_trip["encryption"] is True
+    assert round_trip["query"] == {}
+
+
[email protected]("source", ["toggle", "ssl=1", "ssl_mode=REQUIRED"])
+def test_doris_tls_request_uses_verification(source: str) -> None:
+    """Toggle, scalar SSL and legacy REQUIRED requests use verified TLS."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    if source == "toggle":
+        uri = make_url(
+            DorisEngineSpec.build_sqlalchemy_uri(
+                {
+                    "username": "root",
+                    "host": "localhost",
+                    "port": 9030,
+                    "database": "internal.db",
+                    "encryption": True,
+                }
+            )
+        )
+    else:
+        uri = make_url(f"doris://root@localhost/internal.db?{source}")
+    url, args = DorisEngineSpec.adjust_engine_params(
+        uri, {}, catalog="external", schema="other"
+    )
+    assert args.get("ssl_mode", url.query.get("ssl_mode")) == "VERIFY_CA"
+    assert url.drivername == "doris"
+    assert url.database == "external.other"
+
+
[email protected]("mode", ["DISABLED", "PREFERRED", None])
+def test_doris_toggle_cannot_be_cancelled(mode: Optional[str]) -> None:
+    """Connect arguments cannot cancel a REQUIRED request in the URI."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    with pytest.raises(ValueError, match="conflicts"):
+        DorisEngineSpec.adjust_engine_params(
+            make_url("doris://root@localhost/internal.db?ssl_mode=REQUIRED"),
+            {"ssl_mode": mode},
+        )
+
+
+def test_doris_tls_preserves_verified_mode_and_ca() -> None:
+    """Explicit hostname verification and the native CA dictionary survive."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    uri = 
make_url("doris://root@localhost/internal.db?ssl_mode=VERIFY_IDENTITY")
+    _, args = DorisEngineSpec.adjust_engine_params(uri, {"ssl": {"ca": 
"/ca.pem"}})
+    assert args["ssl_mode"] == "VERIFY_IDENTITY"
+    assert args["ssl"] == {"ca": "/ca.pem"}
+
+
+def test_unrequested_doris_connection_unchanged() -> None:
+    """Connections without a TLS request retain their URI and arguments."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    uri = make_url("doris://root@localhost/internal.db")
+    assert DorisEngineSpec.adjust_engine_params(uri, {}) == (uri, {})
+
+
[email protected]("mode", ["REQUIRED", "VERIFY_CA", "VERIFY_IDENTITY"])
+def test_doris_tls_parameters_round_trip(mode: str) -> None:
+    """Editing TLS URIs keeps encryption enabled and preserves stronger 
modes."""
+    from superset.db_engine_specs.doris import DorisEngineSpec
+
+    parameters = DorisEngineSpec.get_parameters_from_uri(
+        
f"doris://user:p%40ss@localhost:9030/internal.db?ssl_mode={mode}&charset=utf8mb4"
+    )
+    assert parameters["encryption"] is True
+    expected_mode = "VERIFY_IDENTITY" if mode == "VERIFY_IDENTITY" else 
"VERIFY_CA"
+    expected_query = {"charset": "utf8mb4"}
+    if mode == "VERIFY_IDENTITY":
+        expected_query["ssl_mode"] = mode
+    assert parameters["query"] == expected_query
+    uri = make_url(DorisEngineSpec.build_sqlalchemy_uri(parameters))
+    assert uri.query == {"ssl_mode": expected_mode, "charset": "utf8mb4"}
+    assert uri.password == "p@ss"  # noqa: S105
+    assert parameters["encryption"] is True
+    assert parameters["query"] == expected_query

Review Comment:
   These check different states: line 475 calls 
DorisEngineSpec.build_sqlalchemy_uri(parameters) between the first and second 
assertions. The earlier checks validate parsing; the later checks detect a 
builder mutating encryption or query on the caller’s dictionary. Removing them 
loses that regression coverage. They were added in 03306ebaf0 for the 
encryption-flag review finding.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to