diff --git a/linodecli/baked/operation.py b/linodecli/baked/operation.py index 3eb3234e3..a14b2a1e9 100644 --- a/linodecli/baked/operation.py +++ b/linodecli/baked/operation.py @@ -18,7 +18,11 @@ from openapi3.paths import Operation, Parameter from linodecli.baked.parsing import simplify_description -from linodecli.baked.request import OpenAPIFilteringRequest, OpenAPIRequest +from linodecli.baked.request import ( + OpenAPIFilteringRequest, + OpenAPIRequest, + OpenAPIRequestArg, +) from linodecli.baked.response import OpenAPIResponse from linodecli.exit_codes import ExitCodes from linodecli.output.output_handler import OutputHandler @@ -415,6 +419,13 @@ def args(self): """ return self.request.attrs if self.request else [] + @property + def arg_routes(self) -> Dict[str, List[OpenAPIRequestArg]]: + """ + Return a list of attributes from the request schema + """ + return self.request.attr_routes if self.request else [] + @staticmethod def _flatten_url_path(tag: str) -> str: """ diff --git a/linodecli/baked/request.py b/linodecli/baked/request.py index b0bf07340..4023d7434 100644 --- a/linodecli/baked/request.py +++ b/linodecli/baked/request.py @@ -2,7 +2,10 @@ Request details for a CLI Operation """ +from openapi3.schemas import Schema + from linodecli.baked.parsing import simplify_description +from linodecli.baked.util import _aggregate_schema_properties class OpenAPIRequestArg: @@ -134,66 +137,68 @@ def _parse_request_model(schema, prefix=None, parent=None, depth=0): """ args = [] - if schema.properties is not None: - for k, v in schema.properties.items(): - if v.type == "object" and not v.readOnly and v.properties: - # nested objects receive a prefix and are otherwise parsed normally - pref = prefix + "." + k if prefix else k - - args += _parse_request_model( - v, - prefix=pref, + properties, required = _aggregate_schema_properties(schema) + + if properties is None: + return args + + for k, v in properties.items(): + if ( + v.type == "object" + and not v.readOnly + and len(_aggregate_schema_properties(v)[0]) > 0 + ): + # nested objects receive a prefix and are otherwise parsed normally + pref = prefix + "." + k if prefix else k + + args += _parse_request_model( + v, + prefix=pref, + parent=parent, + # NOTE: We do not increment the depth because dicts do not have + # parent arguments. + depth=depth, + ) + elif ( + v.type == "array" + and v.items + and v.items.type == "object" + and v.extensions.get("linode-cli-format") != "json" + ): + # handle lists of objects as a special case, where each property + # of the object in the list is its own argument + pref = prefix + "." + k if prefix else k + + # Support specifying this list as JSON + args.append( + OpenAPIRequestArg( + k, + v.items, + False, + prefix=prefix, + is_parent=True, parent=parent, - # NOTE: We do not increment the depth because dicts do not have - # parent arguments. depth=depth, ) - elif ( - v.type == "array" - and v.items - and v.items.type == "object" - and v.extensions.get("linode-cli-format") != "json" - ): - # handle lists of objects as a special case, where each property - # of the object in the list is its own argument - pref = prefix + "." + k if prefix else k - - # Support specifying this list as JSON - args.append( - OpenAPIRequestArg( - k, - v.items, - False, - prefix=prefix, - is_parent=True, - parent=parent, - depth=depth, - ) - ) + ) - args += _parse_request_model( - v.items, - prefix=pref, - parent=pref, - depth=depth + 1, - ) - else: - # required fields are defined in the schema above the property, so - # we have to check here if required fields are defined/if this key - # is among them and pass it into the OpenAPIRequestArg class. - required = False - if schema.required: - required = k in schema.required - args.append( - OpenAPIRequestArg( - k, - v, - required, - prefix=prefix, - parent=parent, - depth=depth, - ) + args += _parse_request_model( + v.items, + prefix=pref, + parent=pref, + depth=depth + 1, + ) + else: + args.append( + OpenAPIRequestArg( + k, + v, + k in required, + prefix=prefix, + parent=parent, + depth=depth, ) + ) return args @@ -212,15 +217,35 @@ def __init__(self, request): :type request: openapi3.MediaType """ self.required = request.schema.required + schema_override = request.extensions.get("linode-cli-use-schema") + + schema = request.schema + if schema_override: override = type(request)( request.path, {"schema": schema_override}, request._root ) override._resolve_references() - self.attrs = _parse_request_model(override.schema) - else: - self.attrs = _parse_request_model(request.schema) + schema = override.schema + + self.attrs = _parse_request_model(schema) + + # attr_routes stores all attribute routes defined using oneOf. + # For example, config-create uses one of to isolate HTTP, HTTPS, and TCP request attributes + self.attr_routes = {} + + if schema.oneOf is not None: + for entry in schema.oneOf: + entry_schema = Schema(schema.path, entry, request._root) + if entry_schema.title is None: + raise ValueError( + f"No title for oneOf entry in {schema.path}" + ) + + self.attr_routes[entry_schema.title] = _parse_request_model( + entry_schema + ) class OpenAPIFilteringRequest: @@ -249,3 +274,6 @@ def __init__(self, response_model): # actually parse out what we can filter by self.attrs = [c for c in response_model.attrs if c.filterable] + + # This doesn't apply since we're building from the response model + self.attr_routes = {} diff --git a/linodecli/baked/response.py b/linodecli/baked/response.py index 30d9338d4..9d08a7a46 100644 --- a/linodecli/baked/response.py +++ b/linodecli/baked/response.py @@ -4,6 +4,8 @@ from openapi3.paths import MediaType +from linodecli.baked.util import _aggregate_schema_properties + def _is_paginated(response): """ @@ -170,10 +172,12 @@ def _parse_response_model(schema, prefix=None, nested_list_depth=0): attrs = [] - if schema.properties is None: + properties, _ = _aggregate_schema_properties(schema) + + if properties is None: return attrs - for k, v in schema.properties.items(): + for k, v in properties.items(): pref = prefix + "." + k if prefix else k if ( diff --git a/linodecli/baked/util.py b/linodecli/baked/util.py new file mode 100644 index 000000000..fbf9744ae --- /dev/null +++ b/linodecli/baked/util.py @@ -0,0 +1,53 @@ +""" +Provides various utility functions for use in baking logic. +""" + +from collections import defaultdict +from typing import Any, Dict, Set, Tuple + +from openapi3.schemas import Schema + + +def _aggregate_schema_properties( + schema: Schema, +) -> Tuple[Dict[str, Any], Set[str]]: + """ + Aggregates all properties in the given schema, accounting properties + nested in oneOf and anyOf blocks. + + :param schema: The schema to aggregate properties from. + :return: The aggregated properties and a set containing the keys of required properties. + """ + + schema_count = 0 + properties = {} + required = defaultdict(lambda: 0) + + def _handle_schema(_schema: Schema): + if _schema.properties is None: + return + + nonlocal schema_count + schema_count += 1 + + properties.update(dict(_schema.properties)) + + # Aggregate required keys and their number of usages. + if _schema.required is not None: + for key in _schema.required: + required[key] += 1 + + _handle_schema(schema) + + one_of = schema.oneOf or [] + any_of = schema.anyOf or [] + + for entry in one_of + any_of: + # pylint: disable=protected-access + _handle_schema(Schema(schema.path, entry, schema._root)) + + return ( + properties, + # We only want to mark fields that are required by ALL subschema as required + set(key for key, count in required.items() if count == schema_count), + ) diff --git a/linodecli/help_pages.py b/linodecli/help_pages.py index 44e80f461..f73b502b7 100644 --- a/linodecli/help_pages.py +++ b/linodecli/help_pages.py @@ -224,8 +224,13 @@ def print_help_action( _help_action_print_filter_args(console, op) return - if op.args: - _help_action_print_body_args(console, op) + if len(op.arg_routes) > 0: + # This operation uses oneOf so we need to render routes + # instead of the operation-level argument list. + for title, option in op.arg_routes.items(): + _help_action_print_body_args(console, op, option, title=title) + elif op.args: + _help_action_print_body_args(console, op, op.args) def _help_action_print_filter_args(console: Console, op: OpenAPIOperation): @@ -250,13 +255,15 @@ def _help_action_print_filter_args(console: Console, op: OpenAPIOperation): def _help_action_print_body_args( console: Console, op: OpenAPIOperation, + args: List[OpenAPIRequestArg], + title: Optional[str] = None, ): """ Pretty-prints all the body (POST/PUT) arguments for this operation. """ - console.print("[bold]Arguments:[/]") + console.print(f"[bold]Arguments{f' ({title})' if title else ''}:[/]") - for group in _help_group_arguments(op.args): + for group in _help_group_arguments(args): for arg in group: metadata = [] diff --git a/tests/fixtures/operation_with_one_ofs.yaml b/tests/fixtures/operation_with_one_ofs.yaml new file mode 100644 index 000000000..679e04ccb --- /dev/null +++ b/tests/fixtures/operation_with_one_ofs.yaml @@ -0,0 +1,74 @@ +openapi: 3.0.1 +info: + title: API Specification + version: 1.0.0 +servers: + - url: http://localhost/v4 + +paths: + /foo/bar: + x-linode-cli-command: foo + post: + summary: Do something. + operationId: fooBarPost + description: This is description + requestBody: + description: Some description. + required: True + content: + application/json: + schema: + $ref: '#/components/schemas/Foo' + responses: + '200': + description: Successful response + content: + application/json: + schema: + $ref: '#/components/schemas/Foo' + +components: + schemas: + Foo: + oneOf: + - title: Usage 1 + type: object + required: + - foobar + - barfoo + properties: + foobar: + type: string + description: Some foobar. + barfoo: + type: integer + description: Some barfoo. + - title: Usage 2 + type: object + required: + - foobar + - foofoo + properties: + foobar: + type: string + description: Some foobar. + foofoo: + type: boolean + description: Some foofoo. + barbar: + description: Some barbar. + type: object + anyOf: + - type: object + properties: + foo: + type: string + description: Some foo. + bar: + type: integer + description: Some bar. + - type: object + properties: + baz: + type: boolean + description: Some baz. diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index d97b1df60..adc5007ad 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -342,6 +342,25 @@ def get_operation_for_subtable_test(): return make_test_operation(command, operation, method, path.parameters) +@pytest.fixture +def post_operation_with_one_ofs() -> OpenAPIOperation: + """ + Creates a new OpenAPI operation that makes heavy use of oneOfs and anyOfs. + """ + + spec = _get_parsed_spec("operation_with_one_ofs.yaml") + + # Get parameters for OpenAPIOperation() from yaml fixture + path = list(spec.paths.values())[0] + + return make_test_operation( + path.extensions.get("linode-cli-command", "default"), + getattr(path, "post"), + "post", + path.parameters, + ) + + @pytest.fixture def get_openapi_for_api_components_tests() -> OpenAPI: """ diff --git a/tests/unit/test_help_pages.py b/tests/unit/test_help_pages.py index 8709bfd3c..ddb1a4bb7 100644 --- a/tests/unit/test_help_pages.py +++ b/tests/unit/test_help_pages.py @@ -236,3 +236,30 @@ def test_help_command_actions(self, mocker): "Test summary 2.", ], ) + + def test_action_help_post_method_routed( + self, capsys, mocker, mock_cli, post_operation_with_one_ofs + ): + mock_cli.find_operation = mocker.Mock( + return_value=post_operation_with_one_ofs + ) + + help_pages.print_help_action(mock_cli, "command", "action") + captured = capsys.readouterr().out + + print(captured) + + assert "linode-cli command action" in captured + assert "Do something." in captured + + assert "Arguments (Usage 1):" in captured + assert "--foobar (required): Some foobar." in captured + assert "--foofoo (required): Some foofoo." in captured + + assert "Arguments (Usage 2):" in captured + assert "--foobar (required): Some foobar." in captured + assert "--foofoo (required): Some foofoo." in captured + + assert "--barbar.bar: Some bar." in captured + assert "--barbar.baz: Some baz." in captured + assert "--barbar.foo: Some foo." in captured diff --git a/tests/unit/test_request.py b/tests/unit/test_request.py new file mode 100644 index 000000000..b1b4b1926 --- /dev/null +++ b/tests/unit/test_request.py @@ -0,0 +1,23 @@ +class TestRequest: + """ + Unit tests for baked requests. + """ + + def test_handle_one_ofs(self, post_operation_with_one_ofs): + args = post_operation_with_one_ofs.args + + arg_map = {arg.path: arg for arg in args} + + expected = { + "foobar": ("string", "Some foobar.", True), + "barfoo": ("integer", "Some barfoo.", False), + "foofoo": ("boolean", "Some foofoo.", False), + "barbar.foo": ("string", "Some foo.", False), + "barbar.bar": ("integer", "Some bar.", False), + "barbar.baz": ("boolean", "Some baz.", False), + } + + for k, v in expected.items(): + assert arg_map[k].datatype == v[0] + assert arg_map[k].description == v[1] + assert arg_map[k].required == v[2] diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index eb86e3519..b032e00b2 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -1,6 +1,6 @@ -class TestOutputHandler: +class TestResponse: """ - Unit tests for linodecli.response + Unit tests for baked responses. """ def test_model_fix_json_rows(self, list_operation_for_response_test): @@ -46,6 +46,24 @@ def test_attr_render_value(self, list_operation_for_response_test): assert result == "[yellow]cool1, cool2[/]" + def test_handle_one_ofs(self, post_operation_with_one_ofs): + model = post_operation_with_one_ofs.response_model + + attr_map = {attr.path: attr for attr in model.attrs} + + expected = { + "foobar": ("string", "Some foobar"), + "barfoo": ("integer", "Some barfoo"), + "foofoo": ("boolean", "Some foofoo"), + "barbar.foo": ("string", "Some foo"), + "barbar.bar": ("integer", "Some bar"), + "barbar.baz": ("boolean", "Some baz"), + } + + for k, v in expected.items(): + assert attr_map[k].datatype == v[0] + assert attr_map[k].description == v[1] + def test_fix_json_string_type(self, list_operation_for_response_test): model = list_operation_for_response_test.response_model model.rows = ["foo.bar", "type"]