From 274611382ae2971b2c5c7d2292843fa1bcc39b2f Mon Sep 17 00:00:00 2001 From: ezilber-akamai Date: Wed, 9 Sep 2026 11:14:35 -0400 Subject: [PATCH 1/3] Fixed issue with oneof responses getting overwritten during schema aggregation --- linodecli/baked/util.py | 58 +++++++++++++++- .../operation_oneof_property_overwrite.yaml | 68 +++++++++++++++++++ tests/unit/conftest.py | 22 ++++++ tests/unit/test_response.py | 19 ++++++ 4 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/operation_oneof_property_overwrite.yaml diff --git a/linodecli/baked/util.py b/linodecli/baked/util.py index fef1a179c..2626ec604 100644 --- a/linodecli/baked/util.py +++ b/linodecli/baked/util.py @@ -9,6 +9,49 @@ from openapi3.schemas import Schema +def _schema_richness(schema: Any) -> int: + """ + Estimates how "complete" a schema definition is, used to decide which + definition to keep when the same property appears in multiple composition + (oneOf/allOf/anyOf) branches. + + A branch that nulls a property out (e.g. ``{"type": "object", "nullable": + true}`` with no properties) should never overwrite a branch that fully + defines that property's nested structure. + + :param schema: The schema (or raw schema dict) to score. + :return: A non-negative integer; higher means more complete. + """ + + def get(attr: str) -> Any: + if isinstance(schema, dict): + return schema.get(attr) + return getattr(schema, attr, None) + + score = 0 + + if get("properties"): + score += 1 + + if get("oneOf") or get("allOf") or get("anyOf"): + score += 1 + + items = get("items") + if items is not None: + item_get = items.get if isinstance(items, dict) else ( + lambda attr: getattr(items, attr, None) + ) + if ( + item_get("properties") + or item_get("oneOf") + or item_get("allOf") + or item_get("anyOf") + ): + score += 1 + + return score + + def _aggregate_schema_properties( schema: Schema, ) -> Tuple[Dict[str, Any], Set[str]]: @@ -48,7 +91,20 @@ def __inner( return # This is a valid option - properties.update(entry.properties) + for key, value in entry.properties.items(): + # When the same property is defined in multiple composition + # branches (e.g. a oneOf of interface variants that each define + # `public`, `vpc`, `vlan`, etc.), keep the most complete + # definition instead of letting a later, emptier branch overwrite + # it. Otherwise nested fields like `public.ipv6.ranges.range` + # would be silently dropped when a subsequent branch nulls the + # property out. + if key in properties and _schema_richness( + value + ) <= _schema_richness(properties[key]): + continue + + properties[key] = value nonlocal schema_count schema_count += 1 diff --git a/tests/fixtures/operation_oneof_property_overwrite.yaml b/tests/fixtures/operation_oneof_property_overwrite.yaml new file mode 100644 index 000000000..f9757e809 --- /dev/null +++ b/tests/fixtures/operation_oneof_property_overwrite.yaml @@ -0,0 +1,68 @@ +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 + put: + summary: Update something. + operationId: fooBarPut + description: This is description + requestBody: + description: Some description. + required: True + content: + application/json: + schema: + $ref: '#/components/schemas/Interface' + responses: + '200': + description: Successful response + content: + application/json: + schema: + $ref: '#/components/schemas/Interface' + +components: + schemas: + # This schema reproduces the real-world case where a response is a oneOf + # of variants, and every variant defines the SAME set of top-level keys, + # but only fully populates the one relevant to that variant while nulling + # out the others. A naive dict.update() merge lets the last branch + # overwrite the fully-populated definitions from earlier branches. + Interface: + oneOf: + - title: Variant A + type: object + properties: + variant_a: + type: object + properties: + ranges: + type: array + items: + type: object + properties: + range: + type: string + description: The variant A range. + variant_b: + type: object + nullable: true + - title: Variant B + type: object + properties: + variant_a: + type: object + nullable: true + variant_b: + type: object + properties: + label: + type: string + description: The variant B label. + diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 5a6dfc297..f8f9ef991 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -366,6 +366,28 @@ def post_operation_with_one_ofs() -> OpenAPIOperation: ) +@pytest.fixture +def put_operation_with_oneof_property_overwrite() -> OpenAPIOperation: + """ + Creates an OpenAPI operation whose request/response is a oneOf of variants + that each define the same top-level keys, but only fully populate the key + relevant to that variant (nulling the others). Used to verify that + aggregating oneOf branches does not let a later, emptier branch overwrite a + fully-defined property from an earlier branch. + """ + + spec = _get_parsed_spec("operation_oneof_property_overwrite.yaml") + + path = list(spec.paths.values())[0] + + return make_test_operation( + path.extensions.get("linode-cli-command", "default"), + getattr(path, "put"), + "put", + path.parameters, + ) + + @pytest.fixture def get_openapi_for_api_components_tests() -> OpenAPI: """ diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index 91cafca24..2a863ff9b 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -64,6 +64,25 @@ def test_handle_one_ofs(self, post_operation_with_one_ofs): assert attr_map[k].datatype == v[0] assert attr_map[k].description == v[1] + def test_oneof_property_not_overwritten( + self, put_operation_with_oneof_property_overwrite + ): + """ + Regression test: when a response is a oneOf of variants that each define + the same top-level keys (fully populating only one per branch and nulling + the rest), aggregating the branches must not let a later, emptier branch + overwrite a fully-defined property from an earlier branch. + """ + model = put_operation_with_oneof_property_overwrite.response_model + + attr_paths = {attr.path for attr in model.attrs} + + # variant_a is fully defined only in the first branch and nulled in the + # second; its nested field must survive aggregation. + assert "variant_a.ranges.range" in attr_paths + # variant_b is fully defined only in the second branch. + assert "variant_b.label" in attr_paths + 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"] From 5534993f5d6019558d7e5b57c6dbc5ba6dc6f366 Mon Sep 17 00:00:00 2001 From: ezilber-akamai Date: Wed, 9 Sep 2026 11:22:19 -0400 Subject: [PATCH 2/3] Fix lint --- linodecli/baked/util.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/linodecli/baked/util.py b/linodecli/baked/util.py index 2626ec604..6a0b9aff7 100644 --- a/linodecli/baked/util.py +++ b/linodecli/baked/util.py @@ -11,7 +11,7 @@ def _schema_richness(schema: Any) -> int: """ - Estimates how "complete" a schema definition is, used to decide which + Estimates how complete a schema definition is, used to decide which definition to keep when the same property appears in multiple composition (oneOf/allOf/anyOf) branches. @@ -38,8 +38,10 @@ def get(attr: str) -> Any: items = get("items") if items is not None: - item_get = items.get if isinstance(items, dict) else ( - lambda attr: getattr(items, attr, None) + item_get = ( + items.get + if isinstance(items, dict) + else (lambda attr: getattr(items, attr, None)) ) if ( item_get("properties") From d8956689b84bc88d792db8e635140aebf386d853 Mon Sep 17 00:00:00 2001 From: ezilber-akamai Date: Wed, 9 Sep 2026 14:07:24 -0400 Subject: [PATCH 3/3] Fixed oneOf handling in _parse_response_model --- linodecli/baked/response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linodecli/baked/response.py b/linodecli/baked/response.py index b1d3b5f1c..8877d256e 100644 --- a/linodecli/baked/response.py +++ b/linodecli/baked/response.py @@ -210,7 +210,7 @@ def _parse_response_model(schema, prefix=None, nested_list_depth=0): ) elif v.type == "object": attrs += _parse_response_model(v, prefix=pref) - elif v.type == "array" and v.items.type == "object": + elif v.type == "array" and (v.items.type == "object" or v.items.oneOf): # Parse arrays for objects recursively and increase the nesting depth attrs += _parse_response_model( v.items,