From 70dcca12a396ac1b1d889ab48ab3e61b3813a1a1 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Thu, 30 Jan 2025 22:32:19 -0600 Subject: [PATCH 01/14] Added globus auth scope show command --- .../20250130_222824_derek_scope_show.md | 6 + src/globus_cli/commands/__init__.py | 1 + src/globus_cli/commands/auth/__init__.py | 13 ++ .../commands/auth/scope/__init__.py | 11 ++ src/globus_cli/commands/auth/scope/_common.py | 83 +++++++++++++ src/globus_cli/commands/auth/scope/show.py | 78 ++++++++++++ src/globus_cli/login_manager/scopes.py | 3 +- src/globus_cli/services/auth.py | 12 +- src/globus_cli/termio/_display.py | 4 +- src/globus_cli/termio/formatters/__init__.py | 4 + src/globus_cli/termio/formatters/compound.py | 113 +++++++++++++++++- .../termio/printers/record_printer.py | 31 +++-- .../termio/printers/table_printer.py | 4 +- src/globus_cli/types.py | 29 ++++- src/globus_cli/utils.py | 63 +++++++++- tests/conftest.py | 1 + tests/files/api_fixtures/scopes.yaml | 97 +++++++++++++++ .../functional/auth/scope/test_show_scope.py | 71 +++++++++++ .../test_custom_printer.py | 0 .../test_json_printer.py | 0 .../test_record_list_printer.py | 4 +- .../test_record_printer.py | 16 +-- .../test_table_printer.py | 0 tests/unit/test_utils.py | 78 ++++++++++++ 24 files changed, 681 insertions(+), 41 deletions(-) create mode 100644 changelog.d/20250130_222824_derek_scope_show.md create mode 100644 src/globus_cli/commands/auth/__init__.py create mode 100644 src/globus_cli/commands/auth/scope/__init__.py create mode 100644 src/globus_cli/commands/auth/scope/_common.py create mode 100644 src/globus_cli/commands/auth/scope/show.py create mode 100644 tests/files/api_fixtures/scopes.yaml create mode 100644 tests/functional/auth/scope/test_show_scope.py rename tests/unit/termio/{printer => printers}/test_custom_printer.py (100%) rename tests/unit/termio/{printer => printers}/test_json_printer.py (100%) rename tests/unit/termio/{printer => printers}/test_record_list_printer.py (90%) rename tests/unit/termio/{printer => printers}/test_record_printer.py (89%) rename tests/unit/termio/{printer => printers}/test_table_printer.py (100%) diff --git a/changelog.d/20250130_222824_derek_scope_show.md b/changelog.d/20250130_222824_derek_scope_show.md new file mode 100644 index 000000000..85258df64 --- /dev/null +++ b/changelog.d/20250130_222824_derek_scope_show.md @@ -0,0 +1,6 @@ + +### Enhancements + +* Added a new command to display auth scope information: `globus auth scope show`. + + *The "globus auth" command tree is hidden until more commands are added.* diff --git a/src/globus_cli/commands/__init__.py b/src/globus_cli/commands/__init__.py index b960e8055..ef6793f64 100644 --- a/src/globus_cli/commands/__init__.py +++ b/src/globus_cli/commands/__init__.py @@ -4,6 +4,7 @@ @main_group( lazy_subcommands={ "api": ("api", "api_command"), + "auth": ("auth", "auth_command"), "bookmark": ("bookmark", "bookmark_command"), "cli-profile-list": ("cli_profile_list", "cli_profile_list"), "collection": ("collection", "collection_command"), diff --git a/src/globus_cli/commands/auth/__init__.py b/src/globus_cli/commands/auth/__init__.py new file mode 100644 index 000000000..d1f8e4f22 --- /dev/null +++ b/src/globus_cli/commands/auth/__init__.py @@ -0,0 +1,13 @@ +from globus_cli.parsing import group + + +@group( + "auth", + lazy_subcommands={ + "scope": (".scope", "scope_command"), + }, + # TODO - Make the auth command public once we have >= 5 subcommands + hidden=True, +) +def auth_command() -> None: + """Interact with the Globus Auth service.""" diff --git a/src/globus_cli/commands/auth/scope/__init__.py b/src/globus_cli/commands/auth/scope/__init__.py new file mode 100644 index 000000000..c603f910d --- /dev/null +++ b/src/globus_cli/commands/auth/scope/__init__.py @@ -0,0 +1,11 @@ +from globus_cli.parsing import group + + +@group( + "scope", + lazy_subcommands={ + "show": (".show", "show_command"), + }, +) +def scope_command() -> None: + """Interact with a scope in the Globus Auth service.""" diff --git a/src/globus_cli/commands/auth/scope/_common.py b/src/globus_cli/commands/auth/scope/_common.py new file mode 100644 index 000000000..e9aa80f3f --- /dev/null +++ b/src/globus_cli/commands/auth/scope/_common.py @@ -0,0 +1,83 @@ +from __future__ import annotations + +import globus_sdk + +from globus_cli.termio import Field, formatters + +SCOPE_SUMMARY_FIELDS = [ + Field("Scope ID", "scope"), + Field("Optional", "optional", formatter=formatters.Bool), + Field( + "Requires Refresh Token", "requires_refresh_token", formatter=formatters.Bool + ), +] + +DECORATED_SCOPE_SUMMARY_FIELDS = [ + # Scope summaries don't actually contain a "scope_string" field + # But it's very useful to understanding a dependent scope, so we decorate it in. + Field("Scope String", "scope_string"), +] + SCOPE_SUMMARY_FIELDS + +_BASE_SCOPE_FIELDS = [ + Field("Scope String", "scope_string"), + Field("Scope ID", "id"), + Field("Name", "name"), + Field("Description", "description", wrap_enabled=True), + Field("Client ID", "client"), + Field("Allows Refresh Tokens", "allows_refresh_token", formatter=formatters.Bool), + Field("Required Domains", "required_domains", formatter=formatters.Array), + Field("Advertised", "advertised", formatter=formatters.Bool), +] + +DECORATED_SCOPE_FIELDS = _BASE_SCOPE_FIELDS + [ + Field( + "Dependent Scopes", + "dependent_scopes", + formatter=formatters.ArrayMultilineFormatter( + formatters.RecordFormatter(DECORATED_SCOPE_SUMMARY_FIELDS) + ), + ), +] + + +class BatchScopeStringResolver: + """ + A resolver for accessing multiple scope strings without making multiple requests. + + The list of scopes ids, provided at initialization, are queried in a batch request + and cached for future access once the first scope string is resolved. + + :param auth_client: The AuthClient to use for scope string resolution. + :param scope_ids: A list of scope IDs to resolve. + :param default: A default string to return in case a scope id couldn't be found. + """ + + def __init__( + self, + auth_client: globus_sdk.AuthClient, + scope_ids: list[str], + default: str | None = "UNKNOWN", + ) -> None: + self._auth_client = auth_client + self._scope_ids = scope_ids + self._scope_strings: dict[str, str] | None = None + self._default = default + + def resolve(self, scope_id: str) -> str: + if scope_id not in self._scope_ids: + raise ValueError(f"Scope ID {scope_id} was not registered for resolution.") + elif scope_id not in self.scope_strings: + if self._default is not None: + return self._default + raise ValueError(f"Scope string for {scope_id} could not be retrieved.") + return self.scope_strings[scope_id] + + @property + def scope_strings(self) -> dict[str, str]: + """A mapping of scope ID to their scope strings.""" + if self._scope_strings is None: + resp = self._auth_client.get_scopes(ids=self._scope_ids) + self._scope_strings = { + scope["id"]: scope["scope_string"] for scope in resp.get("scopes", []) + } + return self._scope_strings diff --git a/src/globus_cli/commands/auth/scope/show.py b/src/globus_cli/commands/auth/scope/show.py new file mode 100644 index 000000000..c533efb47 --- /dev/null +++ b/src/globus_cli/commands/auth/scope/show.py @@ -0,0 +1,78 @@ +from __future__ import annotations + +import functools +import typing as t + +import click +import globus_sdk + +from globus_cli.login_manager import LoginManager +from globus_cli.parsing import command +from globus_cli.termio import display +from globus_cli.utils import LazyDict, is_uuid + +from ._common import DECORATED_SCOPE_FIELDS, BatchScopeStringResolver + + +@command("show") +@click.argument("SCOPE_ID_OR_STRING", type=str) +@LoginManager.requires_login("auth") +def show_command(login_manager: LoginManager, *, scope_id_or_string: str) -> None: + """Show a scope by ID or string.""" + auth_client = login_manager.get_auth_client() + + if is_uuid(scope_id_or_string): + show_scope_by_id(auth_client, scope_id_or_string) + else: + show_scope_by_string(auth_client, scope_id_or_string) + + +def show_scope_by_id(auth_client: globus_sdk.AuthClient, scope_id: str) -> None: + scope_resp = auth_client.get_scope(scope_id) + + decorate_scope_response(auth_client, scope_resp["scope"]) + + display( + scope_resp, + text_mode=display.RECORD, + fields=DECORATED_SCOPE_FIELDS, + response_key="scope", + ) + + +def show_scope_by_string(auth_client: globus_sdk.AuthClient, scope_string: str) -> None: + scope_resp = auth_client.get_scopes(scope_strings=[scope_string]) + + decorate_scope_response(auth_client, scope_resp["scopes"][0]) + + display( + scope_resp, + text_mode=display.RECORD, + fields=DECORATED_SCOPE_FIELDS, + response_key=lambda resp: resp["scopes"][0], + ) + + +def decorate_scope_response( + auth_client: globus_sdk.AuthClient, + scope: dict[str, t.Any], +) -> None: + """ + Decorates the dependent scopes of a get-scope response. + + Every dependent scope dict has a "scope_string" lazy-loader added to it that will + resolve the scope string by querying globus auth (with batching and caching). + """ + dependent_scopes = scope.get("dependent_scopes") + if not dependent_scopes: + return + + # Create a batch resolver so that we resolve all dependent scope strings at once. + dependent_scope_ids = [dependent["scope"] for dependent in dependent_scopes] + resolver = BatchScopeStringResolver(auth_client, dependent_scope_ids) + + # Replace the dependent scopes with LazyDicts. + scope["dependent_scopes"] = [LazyDict(dependent) for dependent in dependent_scopes] + for dependent in scope["dependent_scopes"]: + load_scope_string = functools.partial(resolver.resolve, dependent["scope"]) + dependent.register_loader("scope_string", load_scope_string) diff --git a/src/globus_cli/login_manager/scopes.py b/src/globus_cli/login_manager/scopes.py index 0d2652a04..597e2b427 100644 --- a/src/globus_cli/login_manager/scopes.py +++ b/src/globus_cli/login_manager/scopes.py @@ -45,7 +45,7 @@ class _ServiceRequirement(t.TypedDict): class _CLIScopeRequirements(t.Dict[ServiceNameLiteral, _ServiceRequirement]): def __init__(self) -> None: self["auth"] = { - "min_contract_version": 0, + "min_contract_version": 1, "resource_server": AuthScopes.resource_server, "nice_server_name": "Globus Auth", "scopes": [ @@ -53,6 +53,7 @@ def __init__(self) -> None: AuthScopes.profile, AuthScopes.email, AuthScopes.view_identity_set, + AuthScopes.manage_projects, ], } self["transfer"] = { diff --git a/src/globus_cli/services/auth.py b/src/globus_cli/services/auth.py index 0b645509d..54af231aa 100644 --- a/src/globus_cli/services/auth.py +++ b/src/globus_cli/services/auth.py @@ -1,18 +1,10 @@ from __future__ import annotations import typing as t -import uuid import globus_sdk -import globus_sdk.scopes - -def _is_uuid(s: str) -> bool: - try: - uuid.UUID(s) - return True - except ValueError: - return False +from globus_cli.utils import is_uuid class GetIdentitiesKwargs(t.TypedDict, total=False): @@ -72,7 +64,7 @@ def maybe_lookup_identity_id( def maybe_lookup_identity_id( self, identity_name: str, provision: bool = False ) -> str | None: - if _is_uuid(identity_name): + if is_uuid(identity_name): return identity_name else: return self._lookup_identity_field( diff --git a/src/globus_cli/termio/_display.py b/src/globus_cli/termio/_display.py index 66131ca11..346c39108 100644 --- a/src/globus_cli/termio/_display.py +++ b/src/globus_cli/termio/_display.py @@ -7,7 +7,6 @@ import globus_sdk from .context import outformat_is_json, outformat_is_text, outformat_is_unix -from .field import Field from .printers import ( CustomPrinter, JsonPrinter, @@ -19,6 +18,9 @@ ) from .server_timing import maybe_show_server_timing +if t.TYPE_CHECKING: + from .field import Field + class TextMode(enum.Enum): silent = enum.auto() diff --git a/src/globus_cli/termio/formatters/__init__.py b/src/globus_cli/termio/formatters/__init__.py index 7f0bd9661..81ee840b5 100644 --- a/src/globus_cli/termio/formatters/__init__.py +++ b/src/globus_cli/termio/formatters/__init__.py @@ -2,7 +2,9 @@ from .base import FieldFormatter, FormattingFailedWarning from .compound import ( ArrayFormatter, + ArrayMultilineFormatter, ParentheticalDescriptionFormatter, + RecordFormatter, SortedJsonFormatter, ) from .primitive import ( @@ -30,6 +32,8 @@ "FuzzyBoolFormatter", "StaticStringFormatter", "ArrayFormatter", + "ArrayMultilineFormatter", + "RecordFormatter", "SortedJsonFormatter", "ParentheticalDescriptionFormatter", "Str", diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index f62a46d55..1346d7b76 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -1,15 +1,30 @@ from __future__ import annotations +import io import json +import os +import textwrap import typing as t -from globus_cli.types import JsonValue +import globus_sdk +from globus_cli.types import JsonDict, JsonValue, is_json_dict + +from ..printers import RecordPrinter from .base import FieldFormatter from .primitive import StrFormatter +if t.TYPE_CHECKING: + from .. import Field + class SortedJsonFormatter(FieldFormatter[JsonValue]): + """ + Example: + in: {"b": 2, "a": 1} + out: '{"a": 1, "b": 2}' + """ + parse_null_values = True def parse(self, value: t.Any) -> JsonValue: @@ -22,6 +37,12 @@ def render(self, value: JsonValue) -> str: class ArrayFormatter(FieldFormatter[t.List[str]]): + """ + Example: + in: ["a", "b", "c"] + out: "a,b,c" + """ + def __init__( self, *, @@ -38,17 +59,105 @@ def __init__( def parse(self, value: t.Any) -> list[str]: if not isinstance(value, list): raise ValueError("non list array value") - data = [self.element_formatter.format(x) for x in value] + data = [self.parse_element(element) for element in value] if self.sort: return sorted(data) else: return data + def parse_element(self, element: t.Any) -> str: + return self.element_formatter.format(element) + def render(self, value: list[str]) -> str: return self.delimiter.join(value) +class ArrayMultilineFormatter(ArrayFormatter): + """ + Example: + in: ["a", "b", "c\nd"] + out: " - a\n - b\n - c\n d" + """ + + def __init__(self, formatter: FieldFormatter[t.Any] | None = None) -> None: + super().__init__(delimiter=os.linesep, sort=False, element_formatter=formatter) + + def render(self, value: list[str]) -> str: + prefix = os.linesep if value else "" + return prefix + super().render(value) + + def parse_element(self, element: t.Any) -> str: + formatted = self.element_formatter.format(element) + return self._indent_element(formatted) + + @classmethod + def _indent_element(cls, value: str) -> str: + """Indent a multi-line formatted element string.""" + first_ = " - " + if not value: + return first_ + indent = " " + indented = textwrap.indent(value, indent, lambda _: True) + return first_ + indented[len(indent) :] + + +Record = t.Union[JsonDict, globus_sdk.GlobusHTTPResponse] + + +class RecordFormatter(FieldFormatter[Record]): + """ + Note: + This formatter is only tested with in a ``RecordPrinter`` and/or an + ``ArrayMultilineFormatter``. + + Additional support for other printers/nested records should be added as needed. + + Example: + in: {"a": 1, "bb": 2} + out: "a: 1\nbb: 2" + """ + + def __init__(self, fields: t.Iterable[Field]) -> None: + self._fields = fields + + def _printer(self, reserved_width: int) -> RecordPrinter: + """ + A RecordPrinter whose max content width is constrained by `reserved_width`. + e.g., if the printer would naturally use a max of 20 chars but 2 are reserved, + it will instead use a max of 18 chars. + """ + max_content_width = RecordPrinter.default_max_content_width() + reduced_max_content_width = max_content_width - reserved_width + return RecordPrinter(self._fields, max_content_width=reduced_max_content_width) + + def parse(self, value: t.Any) -> Record: + if not ( + isinstance(value, globus_sdk.GlobusHTTPResponse) or is_json_dict(value) + ): + raise ValueError("bad RecordFormatter value") + + for field in self._fields: + if field.key not in value: + raise ValueError(f"missing key {field.key} in dict") + return value + + def render(self, value: Record) -> str: + # How do we know how much width to reserve??? + printer = self._printer(reserved_width=2) + + with io.StringIO() as buffer: + printer.echo(value, stream=buffer) + # Materialize the buffer, stripping the trailing newline + return buffer.getvalue().rstrip() + + class ParentheticalDescriptionFormatter(FieldFormatter[t.Tuple[str, str]]): + """ + Example: + in: ["state", "reason for state"] + out: "state (reason for state)" + """ + def parse(self, value: t.Any) -> tuple[str, str]: if not isinstance(value, list) or len(value) != 2: raise ValueError( diff --git a/src/globus_cli/termio/printers/record_printer.py b/src/globus_cli/termio/printers/record_printer.py index 01b7d0928..aa6867ca0 100644 --- a/src/globus_cli/termio/printers/record_printer.py +++ b/src/globus_cli/termio/printers/record_printer.py @@ -10,9 +10,12 @@ from globus_cli.types import JsonValue -from ..field import Field from .base import Printer +if t.TYPE_CHECKING: + from ..field import Field + + DataObject = t.Union[JsonValue, globus_sdk.GlobusHTTPResponse] @@ -28,18 +31,21 @@ class RecordPrinter(Printer[DataObject]): :param fields: a collection of Fields with load and render instructions; one per attribute. - :param max_width: the maximum width of the output. Defaults to 80% of the terminal - width. + :param max_content_width: the maximum width of the output. Defaults to 80% of the + terminal width. """ def __init__( - self, fields: t.Iterable[Field], *, max_width: int | None = None + self, + fields: t.Iterable[Field], + *, + max_content_width: int | None = None, ) -> None: self._fields = list(fields) self._item_wrapper = TextWrapper( initial_indent=" " * self._key_len, subsequent_indent=" " * self._key_len, - width=max_width or _get_terminal_content_width(), + width=max_content_width or self.default_max_content_width(), ) def echo(self, data: DataObject, stream: t.IO[str] | None = None) -> None: @@ -88,6 +94,10 @@ def _key_len(self) -> int: """The number of chars in the key column.""" return max(len(f.name) for f in self._fields) + 2 + @classmethod + def default_max_content_width(self) -> int: + return _get_terminal_content_width() + class RecordListPrinter(Printer[t.Iterable[DataObject]]): """ @@ -105,14 +115,17 @@ class RecordListPrinter(Printer[t.Iterable[DataObject]]): :param fields: a collection of Fields with load and render instructions; one per attribute. - :param max_width: the maximum width of the output. Defaults to 80% of the terminal - width. + :param max_content_width: the maximum width of the output. Defaults to 80% of the + terminal width. """ def __init__( - self, fields: t.Iterable[Field], *, max_width: int | None = None + self, fields: t.Iterable[Field], *, max_content_width: int | None = None ) -> None: - self._record_printer = RecordPrinter(fields, max_width=max_width) + self._record_printer = RecordPrinter( + fields, + max_content_width=max_content_width, + ) def echo( self, diff --git a/src/globus_cli/termio/printers/table_printer.py b/src/globus_cli/termio/printers/table_printer.py index 04957d1d3..e73575c36 100644 --- a/src/globus_cli/termio/printers/table_printer.py +++ b/src/globus_cli/termio/printers/table_printer.py @@ -5,9 +5,11 @@ import click -from ..field import Field from .base import Printer +if t.TYPE_CHECKING: + from ..field import Field + class TablePrinter(Printer[t.Iterable[t.Any]]): """ diff --git a/src/globus_cli/types.py b/src/globus_cli/types.py index 613c5380e..56f046577 100644 --- a/src/globus_cli/types.py +++ b/src/globus_cli/types.py @@ -10,9 +10,9 @@ import click if sys.version_info >= (3, 10): - from typing import TypeAlias + from typing import TypeAlias, TypeGuard else: - from typing_extensions import TypeAlias + from typing_extensions import TypeAlias, TypeGuard # all imports from globus_cli modules done here are done under TYPE_CHECKING # in order to ensure that the use of type annotations never introduces circular @@ -35,9 +35,28 @@ "globus_sdk.GlobusHTTPResponse", ] -JsonValue: TypeAlias = t.Union[ - int, float, str, bool, None, t.List["JsonValue"], t.Dict[str, "JsonValue"] -] + +JsonDict: TypeAlias = t.Dict[str, "JsonValue"] +JsonList: TypeAlias = t.List["JsonValue"] +JsonValue: TypeAlias = t.Union[int, float, str, bool, None, JsonList, JsonDict] + + +def is_json_value(value: t.Any) -> TypeGuard[JsonValue]: + if isinstance(value, (int, float, str, bool, type(None))): + return True + return is_json_list(value) or is_json_dict(value) + + +def is_json_dict(value: t.Any) -> TypeGuard[JsonDict]: + if not isinstance(value, dict): + return False + return all(isinstance(k, str) and is_json_value(v) for k, v in value.items()) + + +def is_json_list(value: t.Any) -> TypeGuard[JsonList]: + if not isinstance(value, list): + return False + return all(is_json_value(item) for item in value) ServiceNameLiteral: TypeAlias = t.Literal[ diff --git a/src/globus_cli/utils.py b/src/globus_cli/utils.py index 8fbc4c316..6a2f8f849 100644 --- a/src/globus_cli/utils.py +++ b/src/globus_cli/utils.py @@ -270,7 +270,7 @@ def resolve_principal_urn( if principal.startswith("urn:globus:groups:id:"): return principal - resolved = principal if _is_uuid(principal) else None + resolved = principal if is_uuid(principal) else None if resolved: return f"urn:globus:groups:id:{resolved}" @@ -284,9 +284,68 @@ def resolve_principal_urn( raise NotImplementedError("unrecognized principal_type") -def _is_uuid(s: str) -> bool: +def is_uuid(s: str) -> bool: try: uuid.UUID(s) return True except ValueError: return False + + +K = t.TypeVar("K") +V = t.TypeVar("V") + + +class LazyDict(dict[K, V]): + """ + A dictionary with support for lazily loaded values. + Lazy loaders are registered using the `register_loader` method. + + Note: This class extends `dict`, not `UserDict` for json serializability. + + Behaviors: + * A lazy-loaded key is contained "in" the dictionary once registered. + * A lazy-loaded key is not included in any presentation of the dictionary unless + it's already been loaded. + * Accessing a lazy-loaded key will load it iff it hasn't been already loaded. + * Otherwise, behaves like a normal dictionary. + """ + + def __init__(self, *args: t.Any, **kwargs: t.Any) -> None: + self._loaders: dict[K, t.Callable[[], V]] = {} + super().__init__(*args, **kwargs) + + def register_loader(self, key: K, load: t.Callable[[], V]) -> None: + """ + Register a callable (() -> Any) with a key. + It will be called once when the key is first accessed to load a value. + """ + self._loaders[key] = load + + def get(self, item: K, default: t.Any = None) -> t.Any: + self._maybe_load_lazy_item(item) + return super().get(item, default) + + def __getitem__(self, item: K) -> V: + self._maybe_load_lazy_item(item) + return super().__getitem__(item) + + def _maybe_load_lazy_item(self, item: K) -> None: + """ + Load a lazy item into the core dictionary if it's not there and has a + registered loading function. + """ + if not super().__contains__(item) and item in self._loaders: + self[item] = self._loaders[item]() + # Remove the loader to prevent reloading if the key is deleted explicitly. + del self._loaders[item] + + def __contains__(self, item: object) -> bool: + return item in self._loaders or super().__contains__(item) + + def __delitem__(self, key: K) -> None: + if key in self._loaders: + del self._loaders[key] + + if key in self: + super().__delitem__(key) diff --git a/tests/conftest.py b/tests/conftest.py index 625ed7251..5e092ba86 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -106,6 +106,7 @@ def mock_login_token_response(): "profile", "email", "urn:globus:auth:scope:auth.globus.org:view_identity_set", + "urn:globus:auth:scope:auth.globus.org:manage_projects", ] ), ), diff --git a/tests/files/api_fixtures/scopes.yaml b/tests/files/api_fixtures/scopes.yaml new file mode 100644 index 000000000..bd6fa4e98 --- /dev/null +++ b/tests/files/api_fixtures/scopes.yaml @@ -0,0 +1,97 @@ + +metadata: + hello_world_id: "24f3dcbe-7655-4721-bc64-d1c5d635b9a1" + hello_world_string: "https://auth.globus.org/scopes/actions.globus.org/hello_world" + +auth: + - path: /v2/api/scopes/24f3dcbe-7655-4721-bc64-d1c5d635b9a1 + method: get + json: + { + "scope": { + "advertised": true, + "allows_refresh_token": true, + "client": "5fac2e64-c734-4e6b-90ea-ff12ddbf9653", + "dependent_scopes": [ + { + "optional": false, + "requires_refresh_token": false, + "scope": "69a73d8f-cd45-4e37-bb3b-43678424aeb7" + }, + { + "optional": false, + "requires_refresh_token": false, + "scope": "73320ffe-4cb4-4b25-a0a3-83d53d59ce4f" + } + ], + "description": "Allow the Hello World action to extend greetings.", + "id": "24f3dcbe-7655-4721-bc64-d1c5d635b9a1", + "name": "Hello World Action", + "required_domains": [], + "scope_string": "https://auth.globus.org/scopes/actions.globus.org/hello_world" + } + } + + - path: /v2/api/scopes + method: get + query_params: + scope_strings: "https://auth.globus.org/scopes/actions.globus.org/hello_world" + json: + { + "scopes": [ + { + "advertised": true, + "allows_refresh_token": true, + "client": "5fac2e64-c734-4e6b-90ea-ff12ddbf9653", + "dependent_scopes": [ + { + "optional": false, + "requires_refresh_token": false, + "scope": "69a73d8f-cd45-4e37-bb3b-43678424aeb7" + }, + { + "optional": false, + "requires_refresh_token": false, + "scope": "73320ffe-4cb4-4b25-a0a3-83d53d59ce4f" + } + ], + "description": "Allow the Hello World action to extend greetings.", + "id": "24f3dcbe-7655-4721-bc64-d1c5d635b9a1", + "name": "Hello World Action", + "required_domains": [ ], + "scope_string": "https://auth.globus.org/scopes/actions.globus.org/hello_world" + } + ] + } + + - path: /v2/api/scopes + method: get + query_params: + ids: "69a73d8f-cd45-4e37-bb3b-43678424aeb7,73320ffe-4cb4-4b25-a0a3-83d53d59ce4f" + json: + { + "scopes": [ + { + "advertised": false, + "allows_refresh_token": true, + "client": "2b2dae68-b536-4acf-9347-cf930e22fa36", + "dependent_scopes": [ ], + "description": "Access to all capabilities of the Globus Groups service.", + "id": "69a73d8f-cd45-4e37-bb3b-43678424aeb7", + "name": "Manage your Globus groups (v1)", + "required_domains": [ ], + "scope_string": "urn:globus:auth:scope:nexus.api.globus.org:groups" + }, + { + "advertised": true, + "allows_refresh_token": true, + "client": "04896e9e-b98e-437e-becd-8084b9e234a0", + "dependent_scopes": [ ], + "description": "View information about groups where you are a member, manager or administrator.", + "id": "73320ffe-4cb4-4b25-a0a3-83d53d59ce4f", + "name": "View Groups and Memberships", + "required_domains": [ ], + "scope_string": "urn:globus:auth:scope:groups.api.globus.org:view_my_groups_and_memberships" + } + ] + } diff --git a/tests/functional/auth/scope/test_show_scope.py b/tests/functional/auth/scope/test_show_scope.py new file mode 100644 index 000000000..d80839a90 --- /dev/null +++ b/tests/functional/auth/scope/test_show_scope.py @@ -0,0 +1,71 @@ +import json +import textwrap + +import pytest +from globus_sdk._testing import load_response_set + + +@pytest.mark.parametrize( + "scope_id_or_string", + ( + "24f3dcbe-7655-4721-bc64-d1c5d635b9a1", + "https://auth.globus.org/scopes/actions.globus.org/hello_world", + ), +) +def test_show_scope(run_line, scope_id_or_string): + load_response_set("cli.scopes") + + result = run_line(f"globus auth scope show {scope_id_or_string}") + + formatter = MultilineFormatter(strip_newlines=(True, False)) + + assert result.stdout == formatter.dedent( + """ + Scope String: https://auth.globus.org/scopes/actions.globus.org/hello_world + Scope ID: 24f3dcbe-7655-4721-bc64-d1c5d635b9a1 + Name: Hello World Action + Description: Allow the Hello World action to extend greetings. + Client ID: 5fac2e64-c734-4e6b-90ea-ff12ddbf9653 + Allows Refresh Tokens: True + Required Domains: + Advertised: True + Dependent Scopes: + - Scope String: urn:globus:auth:scope:nexus.api.globus.org:groups + Scope ID: 69a73d8f-cd45-4e37-bb3b-43678424aeb7 + Optional: False + Requires Refresh Token: False + - Scope String: urn:globus:auth:scope:groups.api.globus.org:view_my_groups_and_memberships + Scope ID: 73320ffe-4cb4-4b25-a0a3-83d53d59ce4f + Optional: False + Requires Refresh Token: False + """ # noqa: E501 + ) + + +def test_show_scope_json_omits_dependent_scope_string(run_line): + meta = load_response_set("cli.scopes").metadata + scope_id = meta["hello_world_id"] + + result = run_line(f"globus auth scope show {scope_id} -F json") + + loaded = json.loads(result.stdout) + assert loaded["scope"]["dependent_scopes"][0].get("scope_string") is None + + +class MultilineFormatter: + def __init__( + self, + strip_newlines: tuple[bool, bool], + line_end_str: str = "", + ) -> None: + self.strip_newlines = strip_newlines + self.line_end_str = line_end_str + + def dedent(self, text: str) -> str: + text = textwrap.dedent(text) + text = text.replace(self.line_end_str, "") + if self.strip_newlines[0]: + text = text.lstrip("\n") + if self.strip_newlines[1]: + text = text.rstrip("\n") + return text diff --git a/tests/unit/termio/printer/test_custom_printer.py b/tests/unit/termio/printers/test_custom_printer.py similarity index 100% rename from tests/unit/termio/printer/test_custom_printer.py rename to tests/unit/termio/printers/test_custom_printer.py diff --git a/tests/unit/termio/printer/test_json_printer.py b/tests/unit/termio/printers/test_json_printer.py similarity index 100% rename from tests/unit/termio/printer/test_json_printer.py rename to tests/unit/termio/printers/test_json_printer.py diff --git a/tests/unit/termio/printer/test_record_list_printer.py b/tests/unit/termio/printers/test_record_list_printer.py similarity index 90% rename from tests/unit/termio/printer/test_record_list_printer.py rename to tests/unit/termio/printers/test_record_list_printer.py index 4ef58b9d9..444f19839 100644 --- a/tests/unit/termio/printer/test_record_list_printer.py +++ b/tests/unit/termio/printers/test_record_list_printer.py @@ -16,7 +16,7 @@ def test_record_list_printer_prints(): {"a": 3, "b": 6, "c": 9}, ) - printer = RecordListPrinter(fields=fields, max_width=80) + printer = RecordListPrinter(fields=fields, max_content_width=80) with StringIO() as stream: printer.echo(data, stream) @@ -47,7 +47,7 @@ def test_record_list_printer_wraps_long_values(): {"a": 2, "b": "b"}, ) - printer = RecordListPrinter(fields=fields, max_width=15) + printer = RecordListPrinter(fields=fields, max_content_width=15) with StringIO() as stream: printer.echo(data, stream) diff --git a/tests/unit/termio/printer/test_record_printer.py b/tests/unit/termio/printers/test_record_printer.py similarity index 89% rename from tests/unit/termio/printer/test_record_printer.py rename to tests/unit/termio/printers/test_record_printer.py index 3f32e9c5c..bc1347905 100644 --- a/tests/unit/termio/printer/test_record_printer.py +++ b/tests/unit/termio/printers/test_record_printer.py @@ -14,7 +14,7 @@ def test_record_printer_prints(): ) data = {"a": 1, "b": 4, "c": 7} - printer = RecordPrinter(fields=fields, max_width=80) + printer = RecordPrinter(fields=fields, max_content_width=80) with StringIO() as stream: printer.echo(data, stream) @@ -37,7 +37,7 @@ def test_record_printer_wraps_long_values(): ) data = {"a": 1, "b": "a" * 40, "c": 7} - printer = RecordPrinter(fields=fields, max_width=25) + printer = RecordPrinter(fields=fields, max_content_width=25) with StringIO() as stream: printer.echo(data, stream) @@ -61,7 +61,7 @@ def test_record_printer_respects_field_wrap_setting(): ) data = {"a": "a" * 10, "b": "b" * 10} - printer = RecordPrinter(fields=fields, max_width=20) + printer = RecordPrinter(fields=fields, max_content_width=20) with StringIO() as stream: printer.echo(data, stream) @@ -80,7 +80,7 @@ def test_record_printer_maintains_data_newlines_when_wrapping(): fields = (Field("Wrapped", "a", wrap_enabled=True),) data = {"a": "a\nbcdefghij"} - printer = RecordPrinter(fields=fields, max_width=15) + printer = RecordPrinter(fields=fields, max_content_width=15) with StringIO() as stream: printer.echo(data, stream) @@ -103,7 +103,7 @@ def test_record_printer_matches_longest_key_length(): ) data = {"a": 1, "b": 4, "c": 7} - printer = RecordPrinter(fields=fields, max_width=80) + printer = RecordPrinter(fields=fields, max_content_width=80) with StringIO() as stream: printer.echo(data, stream) @@ -155,15 +155,15 @@ def test_record_printer_handles_missing_fields(): @pytest.mark.parametrize( - "columns,max_width", + "columns,max_content_width", ( (80, 80), # If the terminal width is > 100, we only use 80% of it. (120, 96), ), ) -def test_record_printer_sets_default_width(monkeypatch, columns, max_width): +def test_record_printer_sets_default_width(monkeypatch, columns, max_content_width): monkeypatch.setenv("COLUMNS", str(columns)) printer = RecordPrinter(fields=(Field("A", "a"),)) - assert printer._item_wrapper.width == max_width + assert printer._item_wrapper.width == max_content_width diff --git a/tests/unit/termio/printer/test_table_printer.py b/tests/unit/termio/printers/test_table_printer.py similarity index 100% rename from tests/unit/termio/printer/test_table_printer.py rename to tests/unit/termio/printers/test_table_printer.py diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index f7ee19821..417b44472 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -5,6 +5,7 @@ from globus_cli.services.auth import CustomAuthClient from globus_cli.utils import ( + LazyDict, format_list_of_words, format_plural_str, resolve_principal_urn, @@ -254,3 +255,80 @@ def test_resolve_principal_urn__when_principal_type_key_is_non_default(): ) assert e.value.message.startswith("'--foobarjohn identity' but") + + +def test_lazy_dict(): + real_dict = {"a": 1, "b": 2} + lazy_dict = LazyDict(real_dict) + + assert "c" not in lazy_dict + assert real_dict == lazy_dict + + lazy_dict.register_loader("c", lambda: 3) + + assert "c" in lazy_dict + assert real_dict == lazy_dict + + assert lazy_dict["c"] == 3 + assert real_dict != lazy_dict + + +def test_lazy_dict_only_loads_once(): + load_call_count = 0 + + def load_data(): + nonlocal load_call_count + load_call_count += 1 + return 3 + + lazy_dict = LazyDict() + lazy_dict.register_loader("c", load_data) + + assert load_call_count == 0 + + assert lazy_dict["c"] == 3 + assert load_call_count == 1 + + assert lazy_dict["c"] == 3 + assert lazy_dict.get("c") == 3 + assert load_call_count == 1 + + +def test_lazy_dict_excludes_loaders_from_presentation_until_loaded(): + lazy_dict = LazyDict({"a": 1, "b": 2}) + lazy_dict.register_loader("c", lambda: 3) + + assert "3" not in repr(lazy_dict) + assert "3" not in str(lazy_dict) + + lazy_dict["c"] + + assert "3" in repr(lazy_dict) + assert "3" in str(lazy_dict) + + +def test_lazy_dict_prefers_explicit_values_to_loaders(): + lazy_dict = LazyDict({"a": 1, "b": 2}) + lazy_dict.register_loader("a", lambda: 3) + + assert lazy_dict["a"] == 1 + + lazy_dict["a"] = 4 + + assert lazy_dict["a"] == 4 + assert lazy_dict.get("a") == 4 + + +def test_lazy_dict_delete_removes_unloaded_loaders(): + lazy = LazyDict({"a": 1}) + lazy.register_loader("a", lambda: 1) + lazy.register_loader("b", lambda: 2) + + assert "a" in lazy + assert "b" in lazy + + del lazy["a"] + del lazy["b"] + + assert "a" not in lazy + assert "b" not in lazy From 9c8e230f5a693609e7a2d8e47c6d79f3df6100e1 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Thu, 30 Jan 2025 22:55:13 -0600 Subject: [PATCH 02/14] Remove list subscripting for py38 --- src/globus_cli/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/globus_cli/utils.py b/src/globus_cli/utils.py index 6a2f8f849..27afab19d 100644 --- a/src/globus_cli/utils.py +++ b/src/globus_cli/utils.py @@ -296,7 +296,7 @@ def is_uuid(s: str) -> bool: V = t.TypeVar("V") -class LazyDict(dict[K, V]): +class LazyDict(t.Dict[K, V]): """ A dictionary with support for lazily loaded values. Lazy loaders are registered using the `register_loader` method. From 914bfbdcedc7db48a9f7d915766bb75e78ea09cc Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Fri, 31 Jan 2025 15:56:13 -0600 Subject: [PATCH 03/14] Added module for tracking global indentation --- src/globus_cli/termio/formatters/compound.py | 22 +++------- .../termio/printers/record_printer.py | 40 ++++++++++--------- src/globus_cli/termio/terminal_info.py | 40 +++++++++++++++++++ .../termio/printers/test_record_printer.py | 34 +++++++--------- tests/unit/termio/test_terminal_info.py | 13 ++++++ tests/unit/termio/test_termio.py | 10 +++-- 6 files changed, 102 insertions(+), 57 deletions(-) create mode 100644 src/globus_cli/termio/terminal_info.py create mode 100644 tests/unit/termio/test_terminal_info.py diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index 1346d7b76..5f452cb1a 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -11,6 +11,7 @@ from globus_cli.types import JsonDict, JsonValue, is_json_dict from ..printers import RecordPrinter +from ..terminal_info import TERM_INFO from .base import FieldFormatter from .primitive import StrFormatter @@ -87,7 +88,8 @@ def render(self, value: list[str]) -> str: return prefix + super().render(value) def parse_element(self, element: t.Any) -> str: - formatted = self.element_formatter.format(element) + with TERM_INFO.indented(4): + formatted = self.element_formatter.format(element) return self._indent_element(formatted) @classmethod @@ -107,7 +109,7 @@ def _indent_element(cls, value: str) -> str: class RecordFormatter(FieldFormatter[Record]): """ Note: - This formatter is only tested with in a ``RecordPrinter`` and/or an + This formatter is only tested within a ``RecordPrinter`` and/or an ``ArrayMultilineFormatter``. Additional support for other printers/nested records should be added as needed. @@ -119,16 +121,7 @@ class RecordFormatter(FieldFormatter[Record]): def __init__(self, fields: t.Iterable[Field]) -> None: self._fields = fields - - def _printer(self, reserved_width: int) -> RecordPrinter: - """ - A RecordPrinter whose max content width is constrained by `reserved_width`. - e.g., if the printer would naturally use a max of 20 chars but 2 are reserved, - it will instead use a max of 18 chars. - """ - max_content_width = RecordPrinter.default_max_content_width() - reduced_max_content_width = max_content_width - reserved_width - return RecordPrinter(self._fields, max_content_width=reduced_max_content_width) + self._printer = RecordPrinter(fields) def parse(self, value: t.Any) -> Record: if not ( @@ -142,11 +135,8 @@ def parse(self, value: t.Any) -> Record: return value def render(self, value: Record) -> str: - # How do we know how much width to reserve??? - printer = self._printer(reserved_width=2) - with io.StringIO() as buffer: - printer.echo(value, stream=buffer) + self._printer.echo(value, stream=buffer) # Materialize the buffer, stripping the trailing newline return buffer.getvalue().rstrip() diff --git a/src/globus_cli/termio/printers/record_printer.py b/src/globus_cli/termio/printers/record_printer.py index aa6867ca0..7a48904c7 100644 --- a/src/globus_cli/termio/printers/record_printer.py +++ b/src/globus_cli/termio/printers/record_printer.py @@ -1,7 +1,6 @@ from __future__ import annotations import functools -import shutil import typing as t from textwrap import TextWrapper @@ -10,6 +9,7 @@ from globus_cli.types import JsonValue +from ..terminal_info import TERM_INFO from .base import Printer if t.TYPE_CHECKING: @@ -31,21 +31,23 @@ class RecordPrinter(Printer[DataObject]): :param fields: a collection of Fields with load and render instructions; one per attribute. - :param max_content_width: the maximum width of the output. Defaults to 80% of the - terminal width. + :param content_width: the maximum width of the output. If omitted, content width + is computed based on the terminal size & globally flagged indentations just + before wrapping strings. """ def __init__( self, fields: t.Iterable[Field], *, - max_content_width: int | None = None, + content_width: int | None = None, ) -> None: self._fields = list(fields) - self._item_wrapper = TextWrapper( + self._is_max_content_width_explicit = content_width is not None + self._base_item_wrapper = TextWrapper( initial_indent=" " * self._key_len, subsequent_indent=" " * self._key_len, - width=max_content_width or self.default_max_content_width(), + width=content_width, ) def echo(self, data: DataObject, stream: t.IO[str] | None = None) -> None: @@ -94,9 +96,18 @@ def _key_len(self) -> int: """The number of chars in the key column.""" return max(len(f.name) for f in self._fields) + 2 - @classmethod - def default_max_content_width(self) -> int: - return _get_terminal_content_width() + @property + def _item_wrapper(self): + """ + Access the printers TextWrapper, modifying the width if necessary. + + :returns: a TextWrapper instance for wrapping item values. + """ + # If the class was instantiated with an explicit max_content_width, don't + # override it. + if not self._is_max_content_width_explicit: + self._base_item_wrapper.width = TERM_INFO.columns + return self._base_item_wrapper class RecordListPrinter(Printer[t.Iterable[DataObject]]): @@ -124,7 +135,7 @@ def __init__( ) -> None: self._record_printer = RecordPrinter( fields, - max_content_width=max_content_width, + content_width=max_content_width, ) def echo( @@ -139,12 +150,3 @@ def echo( prepend_newline = True self._record_printer.echo(item, stream) - - -def _get_terminal_content_width() -> int: - """Get a content width for text output based on the terminal size. - - Uses 80% of the terminal width, if it can be detected and isn't too small. - """ - cols = shutil.get_terminal_size(fallback=(80, 20)).columns - return cols if cols < 100 else int(0.8 * cols) diff --git a/src/globus_cli/termio/terminal_info.py b/src/globus_cli/termio/terminal_info.py new file mode 100644 index 000000000..0f6aa7f46 --- /dev/null +++ b/src/globus_cli/termio/terminal_info.py @@ -0,0 +1,40 @@ +""" +Global interface for managing accessing and modifying terminal info. + +Some printers and formatters within this termio module are concerned with text wrapping. +To do this effectively, they need to know where exactly to start wrapping text. +""" + +import contextlib +import shutil +import typing as t + +__all__ = ("TERM_INFO",) + + +class VirtualTerminalInfo: + MIN_COLUMNS = 6 + + def __init__(self) -> None: + self._column_delta = 0 + cols, rows = shutil.get_terminal_size(fallback=(80, 20)) + self._base_columns = cols if cols < 100 else int(0.8 * cols) + self._base_rows = rows + + @contextlib.contextmanager + def indented(self, size: int) -> t.Iterator[None]: + """ + Context manager to temporarily decrease the available width for text wrapping. + """ + + self._column_delta -= size + yield + self._column_delta += size + + @property + def columns(self) -> int: + computed_columns = self._base_columns + self._column_delta + return max(self.MIN_COLUMNS, computed_columns) + + +TERM_INFO = VirtualTerminalInfo() diff --git a/tests/unit/termio/printers/test_record_printer.py b/tests/unit/termio/printers/test_record_printer.py index bc1347905..b1b899733 100644 --- a/tests/unit/termio/printers/test_record_printer.py +++ b/tests/unit/termio/printers/test_record_printer.py @@ -1,9 +1,8 @@ from io import StringIO -import pytest - from globus_cli.termio import Field from globus_cli.termio.printers import RecordPrinter +from globus_cli.termio.terminal_info import TERM_INFO def test_record_printer_prints(): @@ -14,7 +13,7 @@ def test_record_printer_prints(): ) data = {"a": 1, "b": 4, "c": 7} - printer = RecordPrinter(fields=fields, max_content_width=80) + printer = RecordPrinter(fields=fields, content_width=80) with StringIO() as stream: printer.echo(data, stream) @@ -37,7 +36,7 @@ def test_record_printer_wraps_long_values(): ) data = {"a": 1, "b": "a" * 40, "c": 7} - printer = RecordPrinter(fields=fields, max_content_width=25) + printer = RecordPrinter(fields=fields, content_width=25) with StringIO() as stream: printer.echo(data, stream) @@ -61,7 +60,7 @@ def test_record_printer_respects_field_wrap_setting(): ) data = {"a": "a" * 10, "b": "b" * 10} - printer = RecordPrinter(fields=fields, max_content_width=20) + printer = RecordPrinter(fields=fields, content_width=20) with StringIO() as stream: printer.echo(data, stream) @@ -80,7 +79,7 @@ def test_record_printer_maintains_data_newlines_when_wrapping(): fields = (Field("Wrapped", "a", wrap_enabled=True),) data = {"a": "a\nbcdefghij"} - printer = RecordPrinter(fields=fields, max_content_width=15) + printer = RecordPrinter(fields=fields, content_width=15) with StringIO() as stream: printer.echo(data, stream) @@ -103,7 +102,7 @@ def test_record_printer_matches_longest_key_length(): ) data = {"a": 1, "b": 4, "c": 7} - printer = RecordPrinter(fields=fields, max_content_width=80) + printer = RecordPrinter(fields=fields, content_width=80) with StringIO() as stream: printer.echo(data, stream) @@ -154,16 +153,13 @@ def test_record_printer_handles_missing_fields(): # fmt: on -@pytest.mark.parametrize( - "columns,max_content_width", - ( - (80, 80), - # If the terminal width is > 100, we only use 80% of it. - (120, 96), - ), -) -def test_record_printer_sets_default_width(monkeypatch, columns, max_content_width): - monkeypatch.setenv("COLUMNS", str(columns)) - +def test_record_printer_sets_default_content_width(): + expected_content_width = TERM_INFO.columns printer = RecordPrinter(fields=(Field("A", "a"),)) - assert printer._item_wrapper.width == max_content_width + assert printer._item_wrapper.width == expected_content_width + + +def test_record_printer_respects_explicit_content_width(): + printer = RecordPrinter(fields=(Field("A", "a"),), content_width=5000) + assert TERM_INFO.columns != 5000 + assert printer._item_wrapper.width == 5000 diff --git a/tests/unit/termio/test_terminal_info.py b/tests/unit/termio/test_terminal_info.py new file mode 100644 index 000000000..d233e9035 --- /dev/null +++ b/tests/unit/termio/test_terminal_info.py @@ -0,0 +1,13 @@ +from globus_cli.termio.terminal_info import VirtualTerminalInfo + + +def test_terminal_info_indents(): + term_info = VirtualTerminalInfo() + + columns = term_info.columns + with term_info.indented(4): + assert term_info.columns == columns - 4 + with term_info.indented(4): + assert term_info.columns == columns - 8 + assert term_info.columns == columns - 4 + assert term_info.columns == columns diff --git a/tests/unit/termio/test_termio.py b/tests/unit/termio/test_termio.py index 7a8974f5c..89fdac939 100644 --- a/tests/unit/termio/test_termio.py +++ b/tests/unit/termio/test_termio.py @@ -1,12 +1,12 @@ import os import re import textwrap -from unittest import mock import click import pytest from globus_cli.termio import Field, display, term_is_interactive +from globus_cli.termio.terminal_info import VirtualTerminalInfo @pytest.mark.parametrize( @@ -55,9 +55,13 @@ def test_format_record_list(capsys): def test_format_record_with_text_wrapping(capsys, monkeypatch): # fake the terminal width at 120 - fake_dimensions = mock.Mock() - fake_dimensions.columns = 120 + fake_dimensions = os.terminal_size((120, 20)) monkeypatch.setattr("shutil.get_terminal_size", lambda *_, **__: fake_dimensions) + # Generate a new virtual terminal info object to recompute the columns + monkeypatch.setattr( + "globus_cli.termio.printers.record_printer.TERM_INFO", + VirtualTerminalInfo(), + ) expected_width = int(0.8 * 120) # based on info from wikipedia From 28ef3c55bb974813cc26e7d485ed0ccec5c5a7d1 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Fri, 31 Jan 2025 16:07:49 -0600 Subject: [PATCH 04/14] Make it work for py38 again; someday I'll learn --- tests/functional/auth/scope/test_show_scope.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/functional/auth/scope/test_show_scope.py b/tests/functional/auth/scope/test_show_scope.py index d80839a90..40989d490 100644 --- a/tests/functional/auth/scope/test_show_scope.py +++ b/tests/functional/auth/scope/test_show_scope.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import json import textwrap @@ -17,7 +19,7 @@ def test_show_scope(run_line, scope_id_or_string): result = run_line(f"globus auth scope show {scope_id_or_string}") - formatter = MultilineFormatter(strip_newlines=(True, False)) + formatter = MultilineDedenter(strip_newlines=(True, False)) assert result.stdout == formatter.dedent( """ @@ -52,7 +54,7 @@ def test_show_scope_json_omits_dependent_scope_string(run_line): assert loaded["scope"]["dependent_scopes"][0].get("scope_string") is None -class MultilineFormatter: +class MultilineDedenter: def __init__( self, strip_newlines: tuple[bool, bool], From 5063012536184d90487ae8a355e1442d02da1f14 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Fri, 31 Jan 2025 16:12:00 -0600 Subject: [PATCH 05/14] commit commit commit --- src/globus_cli/termio/printers/record_printer.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/globus_cli/termio/printers/record_printer.py b/src/globus_cli/termio/printers/record_printer.py index 7a48904c7..3dc0c4233 100644 --- a/src/globus_cli/termio/printers/record_printer.py +++ b/src/globus_cli/termio/printers/record_printer.py @@ -47,8 +47,9 @@ def __init__( self._base_item_wrapper = TextWrapper( initial_indent=" " * self._key_len, subsequent_indent=" " * self._key_len, - width=content_width, ) + if content_width is not None: + self._base_item_wrapper.width = content_width def echo(self, data: DataObject, stream: t.IO[str] | None = None) -> None: for field in self._fields: @@ -97,7 +98,7 @@ def _key_len(self) -> int: return max(len(f.name) for f in self._fields) + 2 @property - def _item_wrapper(self): + def _item_wrapper(self) -> TextWrapper: """ Access the printers TextWrapper, modifying the width if necessary. From 5bc3d0f69b54c27de2ddb226a3f0adbcc12d7ef7 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Fri, 31 Jan 2025 17:20:00 -0600 Subject: [PATCH 06/14] Try fixing windows --- tests/functional/auth/scope/test_show_scope.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/auth/scope/test_show_scope.py b/tests/functional/auth/scope/test_show_scope.py index 40989d490..6bb841ff0 100644 --- a/tests/functional/auth/scope/test_show_scope.py +++ b/tests/functional/auth/scope/test_show_scope.py @@ -67,7 +67,7 @@ def dedent(self, text: str) -> str: text = textwrap.dedent(text) text = text.replace(self.line_end_str, "") if self.strip_newlines[0]: - text = text.lstrip("\n") + text = text.lstrip("\r\n") if self.strip_newlines[1]: - text = text.rstrip("\n") + text = text.rstrip("\r\n") return text From eeb9088d89cddc0e6595f4540dea315658d15b23 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 31 Jan 2025 17:49:24 -0600 Subject: [PATCH 07/14] Address Windows-specific output differences --- .../functional/auth/scope/test_show_scope.py | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/tests/functional/auth/scope/test_show_scope.py b/tests/functional/auth/scope/test_show_scope.py index 6bb841ff0..1404068d2 100644 --- a/tests/functional/auth/scope/test_show_scope.py +++ b/tests/functional/auth/scope/test_show_scope.py @@ -19,9 +19,7 @@ def test_show_scope(run_line, scope_id_or_string): result = run_line(f"globus auth scope show {scope_id_or_string}") - formatter = MultilineDedenter(strip_newlines=(True, False)) - - assert result.stdout == formatter.dedent( + expected_output = textwrap.dedent( """ Scope String: https://auth.globus.org/scopes/actions.globus.org/hello_world Scope ID: 24f3dcbe-7655-4721-bc64-d1c5d635b9a1 @@ -29,9 +27,9 @@ def test_show_scope(run_line, scope_id_or_string): Description: Allow the Hello World action to extend greetings. Client ID: 5fac2e64-c734-4e6b-90ea-ff12ddbf9653 Allows Refresh Tokens: True - Required Domains: + Required Domains: Advertised: True - Dependent Scopes: + Dependent Scopes: - Scope String: urn:globus:auth:scope:nexus.api.globus.org:groups Scope ID: 69a73d8f-cd45-4e37-bb3b-43678424aeb7 Optional: False @@ -41,7 +39,12 @@ def test_show_scope(run_line, scope_id_or_string): Optional: False Requires Refresh Token: False """ # noqa: E501 - ) + ).strip() + + # Remove trailing spaces from the command output. + stdout = "\n".join(line.rstrip() for line in result.stdout.splitlines()) + + assert stdout == expected_output def test_show_scope_json_omits_dependent_scope_string(run_line): @@ -52,22 +55,3 @@ def test_show_scope_json_omits_dependent_scope_string(run_line): loaded = json.loads(result.stdout) assert loaded["scope"]["dependent_scopes"][0].get("scope_string") is None - - -class MultilineDedenter: - def __init__( - self, - strip_newlines: tuple[bool, bool], - line_end_str: str = "", - ) -> None: - self.strip_newlines = strip_newlines - self.line_end_str = line_end_str - - def dedent(self, text: str) -> str: - text = textwrap.dedent(text) - text = text.replace(self.line_end_str, "") - if self.strip_newlines[0]: - text = text.lstrip("\r\n") - if self.strip_newlines[1]: - text = text.rstrip("\r\n") - return text From 61c1ab12d327eece38ebdb988cae0496f94e912f Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Mon, 3 Feb 2025 15:21:52 -0600 Subject: [PATCH 08/14] Max recursion depth in json value typeguard --- src/globus_cli/types.py | 44 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/globus_cli/types.py b/src/globus_cli/types.py index 56f046577..5c83722c9 100644 --- a/src/globus_cli/types.py +++ b/src/globus_cli/types.py @@ -42,18 +42,58 @@ def is_json_value(value: t.Any) -> TypeGuard[JsonValue]: + """ + A typeguard for JsonValue. + If the JsonValue is > 100 levels nested deep, it will be considered a JsonValue. + """ + return _is_json_value(value, max_depth=100) + + +def _is_json_value(value: t.Any, max_depth: int) -> TypeGuard[JsonValue]: + if max_depth == 0: + return True + if isinstance(value, (int, float, str, bool, type(None))): return True - return is_json_list(value) or is_json_dict(value) + depth = max_depth - 1 + return _is_json_list(value, max_depth=depth) or _is_json_dict( + value, max_depth=depth + ) def is_json_dict(value: t.Any) -> TypeGuard[JsonDict]: + """ + A typeguard for JsonDict. + If the JsonDict is > 100 levels nested deep, it will be considered a JsonDict. + """ + return _is_json_dict(value, max_depth=100) + + +def _is_json_dict(value: t.Any, max_depth: int) -> TypeGuard[JsonDict]: + if max_depth == 0: + return True + if not isinstance(value, dict): return False - return all(isinstance(k, str) and is_json_value(v) for k, v in value.items()) + depth = max_depth - 1 + return all( + isinstance(k, str) and _is_json_value(v, max_depth=depth) + for k, v in value.items() + ) def is_json_list(value: t.Any) -> TypeGuard[JsonList]: + """ + A typeguard for JsonList. + If the JsonList is > 100 levels nested deep, it will be considered a JsonList. + """ + return _is_json_list(value, max_depth=100) + + +def _is_json_list(value: t.Any, max_depth: int) -> TypeGuard[JsonList]: + if max_depth == 0: + return True + if not isinstance(value, list): return False return all(is_json_value(item) for item in value) From ecf9df7ab0da0f30efc4e2f9574ae87a470d1582 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 19:36:50 -0600 Subject: [PATCH 09/14] Added TerminalTextWrapper --- .../termio/printers/record_printer.py | 22 ++--------- src/globus_cli/termio/terminal_info.py | 32 ++++++++++++++-- tests/unit/termio/test_terminal_info.py | 38 ++++++++++++++++++- tests/unit/termio/test_termio.py | 2 +- 4 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/globus_cli/termio/printers/record_printer.py b/src/globus_cli/termio/printers/record_printer.py index 3dc0c4233..155031bd4 100644 --- a/src/globus_cli/termio/printers/record_printer.py +++ b/src/globus_cli/termio/printers/record_printer.py @@ -2,14 +2,13 @@ import functools import typing as t -from textwrap import TextWrapper import click import globus_sdk from globus_cli.types import JsonValue -from ..terminal_info import TERM_INFO +from ..terminal_info import TerminalTextWrapper from .base import Printer if t.TYPE_CHECKING: @@ -43,13 +42,11 @@ def __init__( content_width: int | None = None, ) -> None: self._fields = list(fields) - self._is_max_content_width_explicit = content_width is not None - self._base_item_wrapper = TextWrapper( + self._item_wrapper = TerminalTextWrapper( initial_indent=" " * self._key_len, subsequent_indent=" " * self._key_len, + width=content_width, ) - if content_width is not None: - self._base_item_wrapper.width = content_width def echo(self, data: DataObject, stream: t.IO[str] | None = None) -> None: for field in self._fields: @@ -97,19 +94,6 @@ def _key_len(self) -> int: """The number of chars in the key column.""" return max(len(f.name) for f in self._fields) + 2 - @property - def _item_wrapper(self) -> TextWrapper: - """ - Access the printers TextWrapper, modifying the width if necessary. - - :returns: a TextWrapper instance for wrapping item values. - """ - # If the class was instantiated with an explicit max_content_width, don't - # override it. - if not self._is_max_content_width_explicit: - self._base_item_wrapper.width = TERM_INFO.columns - return self._base_item_wrapper - class RecordListPrinter(Printer[t.Iterable[DataObject]]): """ diff --git a/src/globus_cli/termio/terminal_info.py b/src/globus_cli/termio/terminal_info.py index 0f6aa7f46..da11a0aca 100644 --- a/src/globus_cli/termio/terminal_info.py +++ b/src/globus_cli/termio/terminal_info.py @@ -5,11 +5,14 @@ To do this effectively, they need to know where exactly to start wrapping text. """ +from __future__ import annotations + import contextlib import shutil import typing as t +from textwrap import TextWrapper -__all__ = ("TERM_INFO",) +__all__ = ("TERM_INFO", "TerminalTextWrapper") class VirtualTerminalInfo: @@ -26,10 +29,11 @@ def indented(self, size: int) -> t.Iterator[None]: """ Context manager to temporarily decrease the available width for text wrapping. """ - self._column_delta -= size - yield - self._column_delta += size + try: + yield + finally: + self._column_delta += size @property def columns(self) -> int: @@ -37,4 +41,24 @@ def columns(self) -> int: return max(self.MIN_COLUMNS, computed_columns) +class TerminalTextWrapper(TextWrapper): + """ + A text wrapper customized for wrapping text to the terminal. + + If width is not supplied, it will be evaluated from ``TERM_INFO``. + """ + + def __init__(self, *args: t.Any, width: int | None = None, **kwargs: t.Any) -> None: + super().__init__(*args, **kwargs) + self._width = width + + @property + def width(self) -> int: + return self._width or TERM_INFO.columns + + @width.setter + def width(self, value: int) -> None: + self._width = value + + TERM_INFO = VirtualTerminalInfo() diff --git a/tests/unit/termio/test_terminal_info.py b/tests/unit/termio/test_terminal_info.py index d233e9035..c8e1d4ee5 100644 --- a/tests/unit/termio/test_terminal_info.py +++ b/tests/unit/termio/test_terminal_info.py @@ -1,4 +1,8 @@ -from globus_cli.termio.terminal_info import VirtualTerminalInfo +from globus_cli.termio.terminal_info import ( + TERM_INFO, + TerminalTextWrapper, + VirtualTerminalInfo, +) def test_terminal_info_indents(): @@ -11,3 +15,35 @@ def test_terminal_info_indents(): assert term_info.columns == columns - 8 assert term_info.columns == columns - 4 assert term_info.columns == columns + + +def test_terminal_info_indentation_is_reset_on_exception(): + term_info = VirtualTerminalInfo() + + columns = term_info.columns + try: + with term_info.indented(4): + assert term_info.columns == columns - 4 + raise ValueError("test") + except ValueError: + pass + assert term_info.columns == columns + + +def test_terminal_text_wrapper_wraps_to_terminal(): + wrapper = TerminalTextWrapper() + assert wrapper.width == TERM_INFO.columns + + a_line = "a" * wrapper.width + assert wrapper.wrap(a_line) == [a_line] + assert wrapper.wrap(a_line + "a") == [a_line, "a"] + + +def test_terminal_text_wrapper_respects_indentation(): + wrapper = TerminalTextWrapper() + + initial_width = TERM_INFO.columns + assert wrapper.width == initial_width + + with TERM_INFO.indented(4): + assert wrapper.width == initial_width - 4 diff --git a/tests/unit/termio/test_termio.py b/tests/unit/termio/test_termio.py index 89fdac939..28b2e2861 100644 --- a/tests/unit/termio/test_termio.py +++ b/tests/unit/termio/test_termio.py @@ -59,7 +59,7 @@ def test_format_record_with_text_wrapping(capsys, monkeypatch): monkeypatch.setattr("shutil.get_terminal_size", lambda *_, **__: fake_dimensions) # Generate a new virtual terminal info object to recompute the columns monkeypatch.setattr( - "globus_cli.termio.printers.record_printer.TERM_INFO", + "globus_cli.termio.terminal_info.TERM_INFO", VirtualTerminalInfo(), ) expected_width = int(0.8 * 120) From 9e8e3dc06c92596bea46e4553dddacf2b8bc8765 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 19:46:16 -0600 Subject: [PATCH 10/14] Comment py38 compatibility decision --- src/globus_cli/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/globus_cli/utils.py b/src/globus_cli/utils.py index 27afab19d..8ee7ccda8 100644 --- a/src/globus_cli/utils.py +++ b/src/globus_cli/utils.py @@ -296,6 +296,8 @@ def is_uuid(s: str) -> bool: V = t.TypeVar("V") +# Note: This class extends `t.Dict` instead of `dict` for python3.8 compatibility. +# Once we drop support for python3.8, we can switch to `dict[K, V]`. class LazyDict(t.Dict[K, V]): """ A dictionary with support for lazily loaded values. From 799ea26854ea32ae7fc1f69f822734868d7c450d Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 19:56:57 -0600 Subject: [PATCH 11/14] ArrayMultilineFormatter._indent_element -> _left_pad_element --- src/globus_cli/termio/formatters/compound.py | 31 +++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index 5f452cb1a..3be525a4b 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -90,17 +90,32 @@ def render(self, value: list[str]) -> str: def parse_element(self, element: t.Any) -> str: with TERM_INFO.indented(4): formatted = self.element_formatter.format(element) - return self._indent_element(formatted) + return self._left_pad_element(formatted) @classmethod - def _indent_element(cls, value: str) -> str: - """Indent a multi-line formatted element string.""" - first_ = " - " + def _left_pad_element(cls, value: str) -> str: + """ + Insert a rectangle of characters to the left of a multiline string. + + Inserted rectangle: + " - " + " " + " " + ... + + Example + " ABC" -> " - ABC" + "DEF" -> " DEF" + " GHI" -> " GHI" + + """ if not value: - return first_ - indent = " " - indented = textwrap.indent(value, indent, lambda _: True) - return first_ + indented[len(indent) :] + return " - " + + # Empty strings are default not indented by textwrap so a predicate is needed. + indented = textwrap.indent(value, " ", predicate=lambda line: True) + + return " - " + indented[4:] Record = t.Union[JsonDict, globus_sdk.GlobusHTTPResponse] From 699bb8d3a78385ac654b1a98f844b9092eea33d5 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 19:59:19 -0600 Subject: [PATCH 12/14] Made left padding example more normal --- src/globus_cli/termio/formatters/compound.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index 3be525a4b..66ad57bb5 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -104,9 +104,9 @@ def _left_pad_element(cls, value: str) -> str: ... Example - " ABC" -> " - ABC" - "DEF" -> " DEF" - " GHI" -> " GHI" + "ABC" -> " - ABC" + "DEF" -> " DEF" + "GHI" -> " GHI" """ if not value: From b56ef116a0055ec7f88aba46aef1995de6181367 Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 20:05:34 -0600 Subject: [PATCH 13/14] Make array render control flow more clear --- src/globus_cli/termio/formatters/compound.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index 66ad57bb5..e764f6572 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -84,8 +84,10 @@ def __init__(self, formatter: FieldFormatter[t.Any] | None = None) -> None: super().__init__(delimiter=os.linesep, sort=False, element_formatter=formatter) def render(self, value: list[str]) -> str: - prefix = os.linesep if value else "" - return prefix + super().render(value) + """Override default array rendering to prepend a newline.""" + if value == "": + return super().render(value) + return os.linesep + super().render(value) def parse_element(self, element: t.Any) -> str: with TERM_INFO.indented(4): From 5e38ed8f71531dee93b34965d52aad0cf37f9e1a Mon Sep 17 00:00:00 2001 From: Derek Schlabach Date: Tue, 4 Feb 2025 20:07:33 -0600 Subject: [PATCH 14/14] Render an empty array as [] --- src/globus_cli/termio/formatters/compound.py | 5 ++++- tests/functional/auth/scope/test_show_scope.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/globus_cli/termio/formatters/compound.py b/src/globus_cli/termio/formatters/compound.py index e764f6572..9f5c78b5f 100644 --- a/src/globus_cli/termio/formatters/compound.py +++ b/src/globus_cli/termio/formatters/compound.py @@ -70,6 +70,9 @@ def parse_element(self, element: t.Any) -> str: return self.element_formatter.format(element) def render(self, value: list[str]) -> str: + if not value: + return "[]" + return self.delimiter.join(value) @@ -85,7 +88,7 @@ def __init__(self, formatter: FieldFormatter[t.Any] | None = None) -> None: def render(self, value: list[str]) -> str: """Override default array rendering to prepend a newline.""" - if value == "": + if not value: return super().render(value) return os.linesep + super().render(value) diff --git a/tests/functional/auth/scope/test_show_scope.py b/tests/functional/auth/scope/test_show_scope.py index 1404068d2..e1ccb584c 100644 --- a/tests/functional/auth/scope/test_show_scope.py +++ b/tests/functional/auth/scope/test_show_scope.py @@ -27,7 +27,7 @@ def test_show_scope(run_line, scope_id_or_string): Description: Allow the Hello World action to extend greetings. Client ID: 5fac2e64-c734-4e6b-90ea-ff12ddbf9653 Allows Refresh Tokens: True - Required Domains: + Required Domains: [] Advertised: True Dependent Scopes: - Scope String: urn:globus:auth:scope:nexus.api.globus.org:groups