diff --git a/sqlit/domains/connections/providers/oracle/adapter.py b/sqlit/domains/connections/providers/oracle/adapter.py index d2a03b74..76edc895 100644 --- a/sqlit/domains/connections/providers/oracle/adapter.py +++ b/sqlit/domains/connections/providers/oracle/adapter.py @@ -77,12 +77,12 @@ def connect(self, config: ConnectionConfig) -> Any: connection_type = config.get_option("oracle_connection_type", "service_name") if connection_type == "sid": - # SID format: host:port:sid (uses colon separator) - # SID is stored in oracle_sid field, fall back to database for backward compat + # Thin-mode Easy Connect doesn't accept the legacy host:port:SID form; + # it tries to resolve the string as a TNS alias and fails with DPY-4027. + # makedsn emits a full TNS descriptor that thin mode handles directly. sid = config.get_option("oracle_sid") or endpoint.database - dsn = f"{endpoint.host}:{port}:{sid}" + dsn = oracledb.makedsn(endpoint.host, port, sid=sid) else: - # Service Name format: host:port/service_name (uses slash separator) dsn = f"{endpoint.host}:{port}/{endpoint.database}" # Determine connection mode based on oracle_role diff --git a/tests/unit/test_oracle_adapter.py b/tests/unit/test_oracle_adapter.py index b1fca942..7693b76e 100644 --- a/tests/unit/test_oracle_adapter.py +++ b/tests/unit/test_oracle_adapter.py @@ -4,6 +4,8 @@ from unittest.mock import MagicMock, patch +import pytest + from tests.helpers import ConnectionConfig @@ -160,7 +162,11 @@ def test_connect_service_name_format(self): assert call_kwargs["dsn"] == "localhost:1521/XEPDB1" def test_connect_sid_format(self): - """Test that sid connection type uses colon separator with oracle_sid field.""" + """SID connection type must go through oracledb.makedsn — see issue #106. + + The legacy host:port:SID Easy-Connect form is rejected by thin-mode with + DPY-4027, so the adapter must use makedsn to emit a TNS descriptor. + """ mock_oracledb = MagicMock() mock_oracledb.AUTH_MODE_SYSDBA = 2 mock_oracledb.AUTH_MODE_SYSOPER = 4 @@ -181,10 +187,9 @@ def test_connect_sid_format(self): adapter.connect(config) - mock_oracledb.connect.assert_called_once() + mock_oracledb.makedsn.assert_called_once_with("localhost", 1521, sid="ORCL") call_kwargs = mock_oracledb.connect.call_args.kwargs - # SID uses colon separator: host:port:sid - assert call_kwargs["dsn"] == "localhost:1521:ORCL" + assert call_kwargs["dsn"] is mock_oracledb.makedsn.return_value def test_connect_sid_backward_compat_uses_database_field(self): """Test that SID falls back to database field for backward compatibility.""" @@ -210,10 +215,7 @@ def test_connect_sid_backward_compat_uses_database_field(self): adapter.connect(config) - mock_oracledb.connect.assert_called_once() - call_kwargs = mock_oracledb.connect.call_args.kwargs - # Should use database field as fallback - assert call_kwargs["dsn"] == "localhost:1521:LEGACY_SID" + mock_oracledb.makedsn.assert_called_once_with("localhost", 1521, sid="LEGACY_SID") def test_connect_default_connection_type_is_service_name(self): """Test that missing oracle_connection_type defaults to service_name format.""" @@ -265,6 +267,34 @@ def test_connect_sid_with_custom_port(self): adapter.connect(config) - mock_oracledb.connect.assert_called_once() - call_kwargs = mock_oracledb.connect.call_args.kwargs - assert call_kwargs["dsn"] == "db.example.com:1522:PROD" + mock_oracledb.makedsn.assert_called_once_with("db.example.com", 1522, sid="PROD") + + def test_sid_dsn_parses_in_real_driver(self): + """Issue #106 regression: adapter must produce a DSN that thin-mode accepts. + + Calls the adapter against a port that is guaranteed closed so the driver + is forced to parse the DSN but cannot complete a network connection. + If parsing fails (DPY-4027), we've regressed. + """ + oracledb = pytest.importorskip("oracledb") + + from sqlit.domains.connections.providers.oracle.adapter import OracleAdapter + + adapter = OracleAdapter() + config = ConnectionConfig( + name="test", + db_type="oracle", + server="127.0.0.1", + port="1", # reserved port — guaranteed no Oracle listener + username="x", + password="x", + options={"oracle_connection_type": "sid", "oracle_sid": "FREE"}, + ) + + with pytest.raises(oracledb.DatabaseError) as exc_info: + adapter.connect(config) + + message = str(exc_info.value) + assert "DPY-4027" not in message, ( + f"issue #106 regression: SID DSN rejected at parse step: {message}" + )