diff --git a/changelog.md b/changelog.md index 88d4e505..04e2a83b 100644 --- a/changelog.md +++ b/changelog.md @@ -4,7 +4,8 @@ Upcoming (TBD) Features --------- * Improve filename completions for zsh. -* Improve acceptance of a DSN alias as a positional argument. +* Improve acceptance of a literal DSN or alias as a positional argument. +* Improve acceptance of a literal DSN or alias as a `--database` argument. * Add bash completions which can complete DSN aliases. * Add fish completions which can complete DSN aliases. * Support Python 3.15 lazy imports for startup performance. diff --git a/mycli/cli_runner.py b/mycli/cli_runner.py index d1f9801b..840c7c31 100644 --- a/mycli/cli_runner.py +++ b/mycli/cli_runner.py @@ -169,35 +169,94 @@ def run_from_cli_args(cli_args: 'CliArgs', client_factory: ClientFactory) -> Non if cli_args.list_dsn: sys.exit(main_list_dsn(mycli)) - # Choose which ever one has a valid value. - database = cli_args.dbname or cli_args.database - dsn_uri = None dsn_password: str | None = None - - # todo why is port tested but not socket? - if database and "://" not in database and database in mycli.config.get("alias_dsn", {}): + database: str | None = '' + explicit_dsn = bool(cli_args.dsn) + + # This could be better written, but it is all made harder by the fact that a DSN is recognized after --database + if ( + cli_args.positional_database + and "://" not in cli_args.positional_database + and cli_args.positional_database in mycli.config.get("alias_dsn", {}) + ): if any([ cli_args.user, cli_args.host, cli_args.port, cli_args.socket, cli_args.login_path, + cli_args.dsn, ]): if cli_verbosity: click.secho( - f'Interpreting ambiguous database/DSN-alias argument "{database}" as a database name.', + f'Interpreting ambiguous positional database/DSN-alias argument "{cli_args.positional_database}" as a database name.', err=True, fg='yellow', ) + database = cli_args.database or cli_args.positional_database else: - if not is_valid_dsn_alias(database): + if not is_valid_dsn_alias(cli_args.positional_database): click.secho(INVALID_DSN_ALIAS_ERROR, err=True, fg='red') sys.exit(1) - cli_args.dsn, database = database, "" + cli_args.dsn, database = cli_args.positional_database, cli_args.database or '' + elif cli_args.positional_database and '://' in cli_args.positional_database: + if cli_args.dsn: + click.secho( + f'Ignoring duplicate positional DSN argument "{cli_args.positional_database}".', + err=True, + fg='yellow', + ) + database = cli_args.database or '' + else: + cli_args.dsn, database = cli_args.positional_database, cli_args.database or '' + elif cli_args.positional_database: + if cli_args.database: + click.secho( + f'Ignoring ambiguous positional database argument "{cli_args.positional_database}" since --database was given.', + err=True, + fg='yellow', + ) + database = cli_args.database + else: + database = cli_args.positional_database + elif cli_args.database: + database = cli_args.database - if database and "://" in database: - dsn_uri, database = database, "" + database_from_option = bool(cli_args.database and database == cli_args.database) + + if database and '://' not in database and database in mycli.config.get('alias_dsn', {}): + if any([ + cli_args.user, + cli_args.host, + cli_args.port, + cli_args.socket, + cli_args.login_path, + cli_args.dsn, + ]): + if database_from_option and (cli_args.verbose or 0) >= 2: + click.secho( + f'Interpreting ambiguous --database argument "{cli_args.database}" as a database name, not a DSN alias, ' + ' since other connection coordinates were given.', + err=True, + fg='yellow', + ) + else: + if not is_valid_dsn_alias(database): + click.secho(INVALID_DSN_ALIAS_ERROR, err=True, fg='red') + sys.exit(1) + cli_args.dsn, database = database, '' + elif database and '://' in database: + if explicit_dsn: + click.secho( + f'Ignoring duplicate DSN argument in --database "{database}".', + err=True, + fg='yellow', + ) + database = '' + else: + cli_args.dsn = '' + dsn_uri, database = database, '' if cli_args.dsn: if not is_valid_dsn_alias(cli_args.dsn): @@ -209,6 +268,9 @@ def run_from_cli_args(cli_args: 'CliArgs', client_factory: ClientFactory) -> Non is_valid_scheme, scheme = is_valid_connection_scheme(cli_args.dsn) if is_valid_scheme: dsn_uri = cli_args.dsn + elif '://' in cli_args.dsn: + click.secho(f'Error: Unknown connection scheme provided for DSN URI ({scheme}://)', err=True, fg='red') + sys.exit(1) else: click.secho( "Could not find the specified DSN in the config file. Please check the \"[alias_dsn]\" section in your myclirc.", diff --git a/mycli/main.py b/mycli/main.py index 93875247..69b58441 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -54,8 +54,9 @@ def convert(self, value, param, ctx): @dataclass(slots=True) class CliArgs: - database: str | None = clickdc.argument( + positional_database: str | None = clickdc.argument( type=str, + metavar='DATABASE', default=None, nargs=1, ) @@ -166,10 +167,9 @@ class CliArgs: is_flag=True, help='Less verbose output and feedback.', ) - dbname: str | None = clickdc.option( + database: str | None = clickdc.option( '-D', '--database', - 'dbname', type=str, clickdc=None, help='Database or DSN to use for the connection.', @@ -401,12 +401,12 @@ def preprocess_cli_args( cli_args: CliArgs, is_valid_connection_scheme: Callable[[str], tuple[bool, str | None]], ) -> int: - if cli_args.database is None and isinstance(cli_args.password, str) and '://' in cli_args.password: + if cli_args.positional_database is None and isinstance(cli_args.password, str) and '://' in cli_args.password: is_valid_scheme, scheme = is_valid_connection_scheme(cli_args.password) if not is_valid_scheme: click.secho(f'Error: Unknown connection scheme provided for DSN URI ({scheme}://)', err=True, fg='red') sys.exit(1) - cli_args.database = cli_args.password + cli_args.positional_database = cli_args.password cli_args.password = EMPTY_PASSWORD_FLAG_SENTINEL if cli_args.resume and not cli_args.checkpoint: diff --git a/test/pytests/test_cli_runner.py b/test/pytests/test_cli_runner.py index 1e161175..927fb5a6 100644 --- a/test/pytests/test_cli_runner.py +++ b/test/pytests/test_cli_runner.py @@ -104,6 +104,20 @@ def test_split_dsn_netloc_handles_bracketed_ipv6_host() -> None: assert cli_runner.split_dsn_netloc('user:pass@[::1]:3306') == ('user', 'pass', '::1', '3306') +@pytest.mark.parametrize( + ('netloc', 'expected'), + ( + ('host', (None, None, 'host', None)), + ('[::1', (None, None, '[', ':1')), + ), +) +def test_split_dsn_netloc_handles_host_without_user_info( + netloc: str, + expected: tuple[str | None, str | None, str | None, str | None], +) -> None: + assert cli_runner.split_dsn_netloc(netloc) == expected + + def test_expand_dsn_alias_env_vars_rejects_non_integer_port(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv('MYCLI_TEST_DSN_PORT', 'not-an-int') @@ -113,6 +127,17 @@ def test_expand_dsn_alias_env_vars_rejects_non_integer_port(monkeypatch: pytest. assert str(excinfo.value) == 'Port in DSN alias prod must be an integer.' +def test_run_from_cli_args_emits_requested_completions(monkeypatch: pytest.MonkeyPatch) -> None: + cli_args = make_cli_args() + cli_args.completions = 'fish' + completion_calls: list[str] = [] + monkeypatch.setattr(cli_runner, 'main_completions', completion_calls.append) + + cli_runner.run_from_cli_args(cli_args, lambda **_kwargs: pytest.fail('client factory called')) + + assert completion_calls == ['fish'] + + def test_run_from_cli_args_checkup_exits_zero(monkeypatch: pytest.MonkeyPatch) -> None: cli_args = make_cli_args() cli_args.checkup = True @@ -159,7 +184,7 @@ def test_run_from_cli_args_rejects_conflicting_format_flags( def test_run_from_cli_args_treats_database_as_dsn_alias(monkeypatch: pytest.MonkeyPatch) -> None: cli_args = make_cli_args() - cli_args.database = 'prod' + cli_args.positional_database = 'prod' client = DummyMyCli( config={ **default_config(), @@ -177,7 +202,7 @@ def test_run_from_cli_args_treats_database_as_dsn_alias(monkeypatch: pytest.Monk assert connect_call['database'] == 'db' -@pytest.mark.parametrize('argument', ['dsn', 'database']) +@pytest.mark.parametrize('argument', ['dsn', 'database', 'positional_database']) def test_run_from_cli_args_rejects_dash_prefixed_dsn_alias( monkeypatch: pytest.MonkeyPatch, argument: str, @@ -207,7 +232,7 @@ def test_run_from_cli_args_reports_ambiguous_database_alias_with_connection_opti monkeypatch: pytest.MonkeyPatch, ) -> None: cli_args = make_cli_args() - cli_args.database = 'prod' + cli_args.positional_database = 'prod' cli_args.user = 'alice' client = DummyMyCli( config={ @@ -222,7 +247,7 @@ def test_run_from_cli_args_reports_ambiguous_database_alias_with_connection_opti assert secho_calls == [ ( - 'Interpreting ambiguous database/DSN-alias argument "prod" as a database name.', + 'Interpreting ambiguous positional database/DSN-alias argument "prod" as a database name.', {'err': True, 'fg': 'yellow'}, ) ] @@ -230,11 +255,213 @@ def test_run_from_cli_args_reports_ambiguous_database_alias_with_connection_opti assert client.connect_calls[-1]['database'] == 'prod' +def test_run_from_cli_args_silently_treats_ambiguous_positional_alias_as_database( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.positional_database = 'prod' + cli_args.user = 'alice' + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://u:p@h/db'}, + } + ) + secho_calls: list[str] = [] + monkeypatch.setattr(main, 'preprocess_cli_args', lambda args, scheme_validator: 0) + monkeypatch.setattr(cli_runner.click, 'secho', lambda text, **_kwargs: secho_calls.append(text)) + monkeypatch.setattr(cli_runner.sys, 'stdin', SimpleNamespace(isatty=lambda: True)) + + cli_runner.run_from_cli_args(cli_args, lambda **_kwargs: client) + + assert secho_calls == [] + assert client.connect_calls[-1]['database'] == 'prod' + + +@pytest.mark.parametrize( + ('positional_database', 'database_option', 'dsn', 'expected_database'), + ( + ('mysql://ignored@ignored/ignored-db', None, 'mysql://user@host/dsn-db', 'dsn-db'), + ('positional-db', 'option-db', None, 'option-db'), + ('positional-db', None, None, 'positional-db'), + ), +) +def test_run_from_cli_args_resolves_positional_database_variants( + monkeypatch: pytest.MonkeyPatch, + positional_database: str, + database_option: str | None, + dsn: str | None, + expected_database: str, +) -> None: + cli_args = make_cli_args() + cli_args.positional_database = positional_database + cli_args.database = database_option + if dsn is not None: + cli_args.dsn = dsn + client = DummyMyCli() + + run_with_client(monkeypatch, cli_args, client) + + assert client.connect_calls[-1]['database'] == expected_database + + +def test_run_from_cli_args_treats_database_option_as_dsn_alias(monkeypatch: pytest.MonkeyPatch) -> None: + cli_args = make_cli_args() + cli_args.database = 'prod' + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://user@host/dsn-db'}, + } + ) + + run_with_client(monkeypatch, cli_args, client) + + assert client.dsn_alias == 'prod' + assert client.connect_calls[-1]['database'] == 'dsn-db' + + +def test_run_from_cli_args_treats_database_option_as_dsn_alias_with_positional_database( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.positional_database = 'positional-db' + cli_args.database = 'prod' + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://user@host/dsn-db'}, + } + ) + + run_with_client(monkeypatch, cli_args, client) + + assert client.dsn_alias == 'prod' + connect_call = client.connect_calls[-1] + assert connect_call['host'] == 'host' + assert connect_call['database'] == 'dsn-db' + + +def test_run_from_cli_args_prefers_database_option_dsn_over_positional_alias( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.positional_database = 'prod' + cli_args.database = 'mysql://option-user@option-host/option-db' + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://alias-user@alias-host/alias-db'}, + } + ) + + run_with_client(monkeypatch, cli_args, client) + + assert client.dsn_alias is None + connect_call = client.connect_calls[-1] + assert connect_call['user'] == 'option-user' + assert connect_call['host'] == 'option-host' + assert connect_call['database'] == 'option-db' + + +def test_run_from_cli_args_treats_ambiguous_database_option_as_database( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.database = 'prod' + cli_args.user = 'alice' + cli_args.verbose = 2 + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://user@host/dsn-db'}, + } + ) + secho_calls: list[str] = [] + monkeypatch.setattr(cli_runner.click, 'secho', lambda text, **_kwargs: secho_calls.append(text)) + + run_with_client(monkeypatch, cli_args, client) + + assert secho_calls == [ + 'Interpreting ambiguous --database argument "prod" as a database name, not a DSN alias, ' + ' since other connection coordinates were given.' + ] + assert client.connect_calls[-1]['database'] == 'prod' + + +def test_run_from_cli_args_silently_treats_ambiguous_database_option_as_database( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.database = 'prod' + cli_args.user = 'alice' + cli_args.verbose = 1 + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://user@host/dsn-db'}, + } + ) + secho_calls: list[str] = [] + monkeypatch.setattr(cli_runner.click, 'secho', lambda text, **_kwargs: secho_calls.append(text)) + + run_with_client(monkeypatch, cli_args, client) + + assert secho_calls == [] + assert client.connect_calls[-1]['database'] == 'prod' + + +@pytest.mark.parametrize( + ('database_option', 'dsn', 'expected_database'), + ( + ('mysql://ignored@ignored/ignored-db', 'mysql://user@host/dsn-db', 'dsn-db'), + ('mysql://user@host/uri-db', None, 'uri-db'), + ), +) +def test_run_from_cli_args_resolves_database_option_dsn_variants( + monkeypatch: pytest.MonkeyPatch, + database_option: str, + dsn: str | None, + expected_database: str, +) -> None: + cli_args = make_cli_args() + cli_args.database = database_option + if dsn is not None: + cli_args.dsn = dsn + client = DummyMyCli() + + run_with_client(monkeypatch, cli_args, client) + + assert client.connect_calls[-1]['database'] == expected_database + + +def test_run_from_cli_args_preserves_connection_options_over_dsn(monkeypatch: pytest.MonkeyPatch) -> None: + cli_args = make_cli_args() + cli_args.database = 'cli-db' + cli_args.dsn = 'mysql://dsn-user@dsn-host:3307/dsn-db?keepalive_ticks=9' + cli_args.user = 'cli-user' + cli_args.host = 'cli-host' + cli_args.port = 3308 + cli_args.keepalive_ticks = 10 + cli_args.format = 'json' + client = DummyMyCli() + + run_with_client(monkeypatch, cli_args, client) + + connect_call = client.connect_calls[-1] + assert cli_args.format == 'json' + assert connect_call['database'] == 'cli-db' + assert connect_call['user'] == 'cli-user' + assert connect_call['host'] == 'cli-host' + assert connect_call['port'] == 3308 + assert connect_call['keepalive_ticks'] == 10 + + def test_run_from_cli_args_allows_dash_prefixed_database_with_connection_options( monkeypatch: pytest.MonkeyPatch, ) -> None: cli_args = make_cli_args() - cli_args.database = '-prod' + cli_args.positional_database = '-prod' cli_args.user = 'alice' client = DummyMyCli( config={ @@ -519,7 +746,7 @@ def test_run_from_cli_args_reports_missing_dsn(monkeypatch: pytest.MonkeyPatch) def test_run_from_cli_args_rejects_unknown_positional_dsn_scheme(monkeypatch: pytest.MonkeyPatch) -> None: cli_args = make_cli_args() - cli_args.database = 'ssh://user@example.com/db' + cli_args.positional_database = 'ssh://user@example.com/db' secho_calls: list[tuple[str, dict[str, Any]]] = [] monkeypatch.setattr(cli_runner.click, 'secho', lambda text, **kwargs: secho_calls.append((text, kwargs))) @@ -1125,6 +1352,25 @@ def test_run_from_cli_args_merges_scalar_global_and_alias_list_init_commands( assert client.connect_calls[-1]['init_command'] == 'set global=1; set alias=1; set alias=2' +def test_run_from_cli_args_ignores_empty_global_and_alias_init_commands( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cli_args = make_cli_args() + cli_args.dsn = 'prod' + client = DummyMyCli( + config={ + **default_config(), + 'alias_dsn': {'prod': 'mysql://u:p@h/db'}, + 'init-commands': {'empty': ''}, + 'alias_dsn.init-commands': {'prod': ''}, + } + ) + + run_with_client(monkeypatch, cli_args, client) + + assert client.connect_calls[-1]['init_command'] == '' + + def test_run_from_cli_args_execute_mode_exits_with_mode_result(monkeypatch: pytest.MonkeyPatch) -> None: cli_args = make_cli_args() cli_args.execute = 'select 1' diff --git a/test/pytests/test_main.py b/test/pytests/test_main.py index 2a7690af..aa9caccc 100644 --- a/test/pytests/test_main.py +++ b/test/pytests/test_main.py @@ -2387,7 +2387,7 @@ def test_preprocess_cli_args_moves_dsn_from_password_to_database() -> None: verbosity = preprocess_cli_args(cli_args, valid_connection_scheme) assert verbosity == 0 - assert cli_args.database == 'mysql://user:pass@host/db' + assert cli_args.positional_database == 'mysql://user:pass@host/db' assert cli_args.password == EMPTY_PASSWORD_FLAG_SENTINEL # type: ignore[comparison-overlap]