diff --git a/changelog.rst b/changelog.rst index 02f8e2ae8..4affd21c6 100644 --- a/changelog.rst +++ b/changelog.rst @@ -24,6 +24,12 @@ Internal: Bug fixes: ---------- +* Fix ``--list-dsn`` and ``-D``/``--dsn`` not finding ``[alias_dsn]`` entries. + On a fresh install (no config written yet) ``--list-dsn`` printed a misleading + "Invalid DSNs found" error. ``--list-dsn`` now treats a missing config or + ``[alias_dsn]`` section as simply nothing to list, without writing a config + file, and ``-D`` resolves the alias from the config already loaded at startup + instead of reading it again ([issue 1489](https://github.com/dbcli/pgcli/issues/1489)). * Restore cursor shape behaviour for Emacs mode * Fix ``TypeError: cannot use a string pattern on a bytes-like object`` when completion metadata comes back as bytes (e.g. ``SQL_ASCII`` client encoding). diff --git a/pgcli/main.py b/pgcli/main.py index 433c25357..a6946fb14 100644 --- a/pgcli/main.py +++ b/pgcli/main.py @@ -1632,9 +1632,14 @@ def cli( config_full_path, ) if list_dsn: + config_file = get_config_filename(pgclirc) + if not os.path.exists(config_file): + # Nothing is configured yet, so there is nothing to list. Don't write + # out the default config just to read it back for a read-only command. + sys.exit(0) try: - cfg = load_config(pgclirc, config_full_path) - for alias in cfg["alias_dsn"]: + cfg = load_config(config_file) + for alias in cfg.get("alias_dsn", {}): click.secho(alias + " : " + cfg["alias_dsn"][alias]) sys.exit(0) except Exception: @@ -1686,7 +1691,9 @@ def cli( if list_databases or ping_database: database = "postgres" - cfg = load_config(pgclirc, config_full_path) + # PGCli() already loaded (and, if needed, wrote) the config above, so reuse it + # rather than reading the file a second time to resolve the -D alias. + cfg = pgcli.config if dsn != "": try: dsn_config = cfg["alias_dsn"][dsn] diff --git a/tests/test_alias_dsn.py b/tests/test_alias_dsn.py new file mode 100644 index 000000000..d1094d2aa --- /dev/null +++ b/tests/test_alias_dsn.py @@ -0,0 +1,90 @@ +import pytest +from click.testing import CliRunner + +from pgcli.main import cli, PGCli + + +def write_config(tmp_path, body): + cfg = tmp_path / "config" + cfg.write_text(body) + return cfg + + +@pytest.fixture +def isolate_config(monkeypatch, tmp_path): + # Keep the real config dir out of the picture. get_config() writes its + # default template under here when a config file does not exist yet. + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) + return tmp_path + + +def test_list_dsn_fresh_config(isolate_config): + # No config file exists yet. --list-dsn used to read the not-yet-written + # config before PGCli() created it, hit a KeyError on the missing + # [alias_dsn] section, and print "Invalid DSNs found" with exit code 1. + # It should just report no aliases and exit cleanly. + cfg = isolate_config / "config" + runner = CliRunner() + result = runner.invoke(cli, ["--pgclirc", str(cfg), "--list-dsn"]) + assert result.exit_code == 0 + assert "Invalid DSNs" not in result.output + assert result.output.strip() == "" + # A read-only command must not create the config file as a side effect. + assert not cfg.exists() + + +def test_list_dsn_lists_aliases(isolate_config): + cfg = write_config( + isolate_config, + "[alias_dsn]\nfoo = postgres://u:p@localhost:5432/foo\nbar = postgres://u:p@localhost:5432/bar\n", + ) + runner = CliRunner() + result = runner.invoke(cli, ["--pgclirc", str(cfg), "--list-dsn"]) + assert result.exit_code == 0 + assert "foo : postgres://u:p@localhost:5432/foo" in result.output + assert "bar : postgres://u:p@localhost:5432/bar" in result.output + + +def test_dsn_alias_resolves(isolate_config, monkeypatch): + cfg = write_config( + isolate_config, + "[alias_dsn]\nfoo = postgres://u:p@localhost:5432/foo\n", + ) + captured = {} + + def fake_connect(self, *args, **kwargs): + # connect_uri() parses the alias URI and calls connect() with the + # resolved parts, so recording them proves the alias was found. + captured.update(kwargs) + + class DummyExec: + def run(self, cmd): + return [] + + def get_timezone(self): + return "UTC" + + def set_timezone(self, *a, **k): + pass + + self.pgexecute = DummyExec() + + monkeypatch.setattr(PGCli, "connect", fake_connect) + + runner = CliRunner() + result = runner.invoke(cli, ["--pgclirc", str(cfg), "-D", "foo", "--ping"]) + assert result.exit_code == 0 + assert "Could not find a DSN" not in result.output + assert captured.get("database") == "foo" + assert captured.get("host") == "localhost" + + +def test_dsn_alias_missing(isolate_config): + cfg = write_config( + isolate_config, + "[alias_dsn]\nfoo = postgres://u:p@localhost:5432/foo\n", + ) + runner = CliRunner() + result = runner.invoke(cli, ["--pgclirc", str(cfg), "-D", "does-not-exist"]) + assert result.exit_code == 1 + assert "Could not find a DSN with alias does-not-exist" in result.output