After upgrading from 2.4.1 to 3.3.0, generated swagger schemas are incorrect for `fields.List(fields.Nested(...), allow_none=True)`

[kaiku]

Problem

The latest version of flask-rebar generates a swagger.json that "expands" $ref to nested schemas specifically when using fields.List(fields.Nested(...), allow_none=True).

Details

I have marshmallow schemas with fields that look like this:

class NestedSchema(Schema):
    foo = fields.String()

class ParentSchema(Schema):
    data = fields.List(fields.Nested(NestedSchema), required=False, allow_none=True)

In 2.4.1, using swagger_generator_v3 produces a swagger.json that looks like this:

"ParentSchema": {
  "additionalProperties": false,
  "properties": {
    "data": {
      "items": {
        "$ref": "#/components/schemas/NestedSchema"
      },
      "nullable": true,
      "type": "array"
    }
  },
  "title": "ParentSchema",
  "type": "object"
}

In 3.3.0, it looks like this:

"ParentSchema": {
  "additionalProperties": false,
  "properties": {
    "data": {
      "items": {
        "additionalProperties": false,
        "description": "A nested schema",
        "properties": {
          "foo": {
            "type": "string"
          },
        },
        "required": [
          "foo"
        ],
        "title": "NestedSchema",
        "type": "object"
      },
      "type": [
        "array",
        "null"
      ]
    }
  },
  "title": "ParentSchema",
  "type": "object"
}

Expected:

"ParentSchema": {
  "additionalProperties": false,
  "properties": {
    "data": {
      "items": {
        "$ref": "#/components/schemas/NestedSchema"
      },
        "required": [
          "foo"
        ],
        "title": "NestedSchema",
        "type": "object"
      },
      "type": [
        "array",
        "null"
      ]
    }
  },
  "title": "ParentSchema",
  "type": "object"
}

The important difference here is that instead of a $ref, the swagger codegen now expands NestedSchema into an embedded dict. From a raw API perspective there is no functional difference in the spec, but this does cause problems in our unit tests because we use a python client generated by swagger-codegen-cli.jar to make calls against our service, and responses from that are different.

Investigation

allow_none=True seems to be the culprit. If I remove that line, I get a nice ref again. However, I can't do this because it would break our API contract of allowing that field to actually be None.

I've tried both fields.List(fields.Nested(...)) and fields.Nested(..., many=True). They seem similar but not equivalent. If I use the latter, it does create a $ref again, but null is missing from the type list, which breaks the API contract. I also tried fields.Nested(..., many=True, allow_none=True), which I do think is valid and marshmallow supports it, but the generated schema still does not reflect that it can be a nullable field.

[kaiku]

When type is a list, as it will be with OpenAPI 3.1 if none is allowed, this will fail:

https://github.com/plangrid/flask-rebar/blob/ffd15425463f90c24ed7829e6b26b258a1ece852/flask_rebar/swagger_generation/generator_utils.py#L146-L161

The consequence of this is that _flatten is not called recursively for type: [...] and nested objects are not resolved into refs.

Here is where type is made into either a string or list of strings:

https://github.com/plangrid/flask-rebar/blob/ffd15425463f90c24ed7829e6b26b258a1ece852/flask_rebar/swagger_generation/marshmallow_to_swagger.py#L349-L357

[kaiku]

I've added a fix in: https://github.com/plangrid/flask-rebar/pull/315/commits/f70f5b562253950aca3218c457746486ad9fbe9b. Needs updates to tests first.