diff --git a/linodecli/api_request.py b/linodecli/api_request.py index 4273aa6d2..c61c47632 100644 --- a/linodecli/api_request.py +++ b/linodecli/api_request.py @@ -7,7 +7,7 @@ import sys import time from sys import version_info -from typing import Iterable, List, Optional +from typing import Any, Iterable, List, Optional import requests from packaging import version @@ -15,7 +15,11 @@ from linodecli.helpers import API_CA_PATH -from .baked.operation import ExplicitNullValue, OpenAPIOperation +from .baked.operation import ( + ExplicitEmptyListValue, + ExplicitNullValue, + OpenAPIOperation, +) from .helpers import handle_url_overrides @@ -199,6 +203,47 @@ def _build_request_url(ctx, operation, parsed_args) -> str: return result +def _traverse_request_body(o: Any) -> Any: + """ + This function traverses is intended to be called immediately before + request body serialization and contains special handling for dropping + keys with null values and translating ExplicitNullValue instances into + serializable null values. + """ + if isinstance(o, dict): + result = {} + for k, v in o.items(): + # Implicit null values should be dropped from the request + if v is None: + continue + + # Values that are expected to be serialized as empty lists + # and explicit None values are converted here. + # See: operation.py + # NOTE: These aren't handled at the top-level of this function + # because we don't want them filtered out in the step below. + if isinstance(v, ExplicitEmptyListValue): + result[k] = [] + continue + + if isinstance(v, ExplicitNullValue): + result[k] = None + continue + + value = _traverse_request_body(v) + + # We should exclude implicit empty lists + if not (isinstance(value, (dict, list)) and len(value) < 1): + result[k] = value + + return result + + if isinstance(o, list): + return [_traverse_request_body(v) for v in o] + + return o + + def _build_request_body(ctx, operation, parsed_args) -> Optional[str]: if operation.method == "get": # Get operations don't have a body @@ -208,24 +253,10 @@ def _build_request_body(ctx, operation, parsed_args) -> Optional[str]: if ctx.defaults: parsed_args = ctx.config.update(parsed_args, operation.allowed_defaults) - to_json = {} - - for k, v in vars(parsed_args).items(): - # Skip null values - if v is None: - continue - - # Explicitly include ExplicitNullValues - if isinstance(v, ExplicitNullValue): - to_json[k] = None - continue - - to_json[k] = v - expanded_json = {} # expand paths - for k, v in to_json.items(): + for k, v in vars(parsed_args).items(): cur = expanded_json for part in k.split(".")[:-1]: if part not in cur: @@ -233,7 +264,7 @@ def _build_request_body(ctx, operation, parsed_args) -> Optional[str]: cur = cur[part] cur[k.split(".")[-1]] = v - return json.dumps(expanded_json) + return json.dumps(_traverse_request_body(expanded_json)) def _print_request_debug_info(method, url, headers, body): diff --git a/linodecli/baked/operation.py b/linodecli/baked/operation.py index 289f22b3a..db1c484d4 100644 --- a/linodecli/baked/operation.py +++ b/linodecli/baked/operation.py @@ -60,6 +60,12 @@ class ExplicitNullValue: """ +class ExplicitEmptyListValue: + """ + A special type used to explicitly pass empty lists to the API. + """ + + def wrap_parse_nullable_value(arg_type): """ A helper function to parse `null` as None for nullable CLI args. @@ -91,9 +97,16 @@ def __call__(self, parser, namespace, values, option_string=None): output_list = getattr(namespace, self.dest) + # If a user has already specified an [] but is specifying + # another value, assume "[]" was intended to be a literal. + if isinstance(output_list, ExplicitEmptyListValue): + setattr(namespace, self.dest, ["[]", values]) + return + # If the output list is empty and the user specifies a [] - # argument, keep the list empty + # argument, set the list to an explicitly empty list. if values == "[]" and len(output_list) < 1: + setattr(namespace, self.dest, ExplicitEmptyListValue()) return output_list.append(values) diff --git a/tests/fixtures/api_request_test_foobar_post.yaml b/tests/fixtures/api_request_test_foobar_post.yaml index 401b7bfba..f7dd8d934 100644 --- a/tests/fixtures/api_request_test_foobar_post.yaml +++ b/tests/fixtures/api_request_test_foobar_post.yaml @@ -120,3 +120,7 @@ components: field_int: type: number description: An arbitrary field. + nullable_string: + type: string + description: An arbitrary nullable string. + nullable: true diff --git a/tests/unit/test_api_request.py b/tests/unit/test_api_request.py index 167a2e087..6e4cade6d 100644 --- a/tests/unit/test_api_request.py +++ b/tests/unit/test_api_request.py @@ -12,7 +12,7 @@ import requests from linodecli import api_request -from linodecli.baked.operation import ExplicitNullValue +from linodecli.baked.operation import ExplicitEmptyListValue, ExplicitNullValue class TestAPIRequest: @@ -102,6 +102,36 @@ def test_build_request_body_null_field(self, mock_cli, create_operation): == result ) + def test_build_request_body_null_field_nested( + self, mock_cli, create_operation + ): + create_operation.allowed_defaults = ["region", "image"] + result = api_request._build_request_body( + mock_cli, + create_operation, + SimpleNamespace( + generic_arg="foo", + region=None, + image=None, + object_list=[{"nullable_string": ExplicitNullValue()}], + ), + ) + assert ( + json.dumps( + { + "generic_arg": "foo", + "region": "us-southeast", + "image": "linode/ubuntu21.10", + "object_list": [ + { + "nullable_string": None, + } + ], + } + ) + == result + ) + def test_build_request_body_non_null_field( self, mock_cli, create_operation ): @@ -517,3 +547,41 @@ def test_get_retry_after(self): headers = {} output = api_request._get_retry_after(headers) assert output == 0 + + def test_traverse_request_body(self): + result = api_request._traverse_request_body( + { + "test_string": "sdf", + "test_obj_list": [ + { + "populated_field": "cool", + "foo": None, + "bar": ExplicitNullValue(), + } + ], + "test_dict": { + "foo": "bar", + "bar": None, + "baz": ExplicitNullValue(), + }, + "cool": [], + "cooler": ExplicitEmptyListValue(), + "coolest": ExplicitNullValue(), + } + ) + + assert result == { + "test_string": "sdf", + "test_obj_list": [ + { + "populated_field": "cool", + "bar": None, + } + ], + "test_dict": { + "foo": "bar", + "baz": None, + }, + "cooler": [], + "coolest": None, + } diff --git a/tests/unit/test_operation.py b/tests/unit/test_operation.py index 68b153da7..2f8d518c6 100644 --- a/tests/unit/test_operation.py +++ b/tests/unit/test_operation.py @@ -1,7 +1,7 @@ import argparse from linodecli.baked import operation -from linodecli.baked.operation import ExplicitNullValue +from linodecli.baked.operation import ExplicitEmptyListValue, ExplicitNullValue class TestOperation: @@ -178,11 +178,13 @@ def test_parse_args_object_list(self, create_operation): "test3", ] ) + assert result.object_list == [ { "field_string": "test1", "field_int": 123, "field_dict": {"nested_string": "test2", "nested_int": 789}, + "nullable_string": None, # We expect this to be filtered out later }, {"field_int": 456, "field_dict": {"nested_string": "test3"}}, ] @@ -209,7 +211,7 @@ def test_array_arg_action_basic(self): # User wants an explicitly empty list result = parser.parse_args(["--foo", "[]"]) - assert getattr(result, "foo") == [] + assert isinstance(getattr(result, "foo"), ExplicitEmptyListValue) # User doesn't specify the list result = parser.parse_args([])