Fix several issues with oneof responses

linodecli/baked/response.py

@@ -210,8 +210,10 @@ 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":
-            # Parse arrays for objects recursively and increase the nesting depth
+        elif v.type == "array" and (
+            v.items.type == "object"
+            or bool(_aggregate_schema_properties(v.items)[0])
+        ):
             attrs += _parse_response_model(
                 v.items,
                 prefix=pref,

linodecli/baked/util.py

@@ -8,6 +8,67 @@
 
 from openapi3.schemas import Schema
 
+# The maximum schema nesting depth `_schema_richness` will traverse before
+# bailing out. This is purely a safety valve to guarantee termination on
+# self-referential or pathologically deep schemas (which the recursion would
+# otherwise follow forever); it is far deeper than any real Linode API response
+# model, so it never affects scoring in practice.
+_MAX_RICHNESS_DEPTH = 32
+
+
+def _schema_richness(schema: Any, _depth: int = 0) -> 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. The score is a recursive measure
+    of how much structure a schema actually contains, so a fuller definition always outscores a
+    sparser one regardless of the
+    order the branches appear in.
+
+    :param schema: The schema (or raw schema dict) to score.
+    :return: A non-negative integer; higher means more complete.
+    """
+
+    # Guard against pathologically deep or self-referential schemas.
+    if _depth > _MAX_RICHNESS_DEPTH:
+        return 0
+
+    def get(source: Any, attr: str) -> Any:
+        if isinstance(source, dict):
+            return source.get(attr)
+        return getattr(source, attr, None)
+
+    score = 0
+
+    # Count each defined property, plus the richness of its own definition so
+    # that deeply-nested structure contributes to the total.
+    properties = get(schema, "properties")
+    if properties:
+        for _, prop in properties.items():
+            score += 1 + _schema_richness(prop, _depth + 1)
+
+    # Account for composite (oneOf/allOf/anyOf) definitions by summing the
+    # richness of each branch.
+    for composition_field in ("oneOf", "allOf", "anyOf"):
+        for branch in get(schema, composition_field) or []:
+            score += 1 + _schema_richness(branch, _depth + 1)
+
+    # Account for array item schemas so arrays of objects are scored by their
+    # element structure.
+    array_items = get(schema, "items")
+    if array_items is not None:
+        score += _schema_richness(array_items, _depth + 1)
+
+    additional_properties = get(schema, "additionalProperties")
+    if additional_properties is not None:
+        score += 1 + _schema_richness(additional_properties, _depth + 1)
+
+    return score
+
 
 def _aggregate_schema_properties(
     schema: Schema,
@@ -48,7 +109,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

tests/fixtures/operation_oneof_property_overwrite.yaml

@@ -0,0 +1,97 @@
+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
+            # An array whose items are a oneOf of SCALAR types. This must be
+            # treated as a normal array attribute, not recursed into.
+            scalar_choices:
+              type: array
+              items:
+                oneOf:
+                  - type: string
+                  - type: integer
+            # This shared object is defined more richly in the LATER branch
+            # The fuller definition must win regardless of branch
+            # order, so all of its nested fields must survive aggregation.
+            shared_obj:
+              type: object
+              properties:
+                only_a:
+                  type: string
+                  description: Present in both branches.
+        - 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.
+            shared_obj:
+              type: object
+              properties:
+                only_a:
+                  type: string
+                  description: Present in both branches.
+                extra_b1:
+                  type: string
+                  description: Only defined in the richer Variant B branch.
+                extra_b2:
+                  type: string
+                  description: Only defined in the richer Variant B branch.
+

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:
     """

tests/unit/test_response.py

@@ -118,6 +118,60 @@ 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_scalar_oneof_array_not_dropped(
+        self, put_operation_with_oneof_property_overwrite
+    ):
+        """
+        Regression test: an array whose items are a oneOf of scalar types
+        (e.g. ``items: {oneOf: [{type: string}, {type: integer}]}``) aggregates
+        no object properties. It must remain a normal array attribute instead of
+        being recursed into and silently dropped from the response model.
+        """
+        model = put_operation_with_oneof_property_overwrite.response_model
+
+        attr_paths = {attr.path for attr in model.attrs}
+
+        assert "scalar_choices" in attr_paths
+
+    def test_richer_oneof_branch_wins_regardless_of_order(
+        self, put_operation_with_oneof_property_overwrite
+    ):
+        """
+        Regression test: when the same object property is defined in multiple
+        branches, the branch with the most nested structure must win even if it
+        appears later. A presence-only richness score would tie and keep the
+        earlier, sparser definition, dropping the extra fields.
+        """
+        model = put_operation_with_oneof_property_overwrite.response_model
+
+        attr_paths = {attr.path for attr in model.attrs}
+
+        # Defined in both branches.
+        assert "shared_obj.only_a" in attr_paths
+        # Only defined in the richer (later) Variant B branch; these would be
+        # missing if the earlier, sparser definition were kept.
+        assert "shared_obj.extra_b1" in attr_paths
+        assert "shared_obj.extra_b2" 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"]