Extra allOf keys in translation of Unions to OpenAPI schema when Field is used

[chriswmackey]

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

pydantic version: 1.4
            pydantic compiled: False
                 install path: C:\Users\chris\AppData\Local\Programs\Python\Python37-32\Lib\site-packages\pydantic
               python version: 3.7.1 (v3.7.1:260ec2c36a, Oct 20 2018, 14:05:16) [MSC v.1915 32 bit (Intel)]
                     platform: Windows-10-10.0.18362-SP0
     optional deps. installed: []

Description

My teammates and I debated this for a bit but we're fairly confident that we're experiencing a bug, though it could just be a misunderstanding of how the translation to OpenAPI schema is intended to happen. We can at least say that we couldn't find any other issues on this github addressing it or samples in the docs about our case.

Specifically, when we run the following sample:

from pydantic import BaseModel, Field
from typing import Union
from pprint import pprint

class TestClassA(BaseModel):
    pass

class TestClassB(BaseModel):
    pass

class TestClassC(BaseModel):
    a: Union[TestClassA, TestClassB]

pprint(TestClassC.schema())

... we get the expected result:

{'definitions': {'TestClassA': {'properties': {},
                                'title': 'TestClassA',
                                'type': 'object'},
                 'TestClassB': {'properties': {},
                                'title': 'TestClassB',
                                'type': 'object'}},
 'properties': {'a': {'anyOf': [{'$ref': '#/definitions/TestClassA'},
                                {'$ref': '#/definitions/TestClassB'}],
                      'title': 'A'}},
 'required': ['a'],
 'title': 'TestClassC',
 'type': 'object'}

However, when we change TestClassC to be the following:

class TestClassC(BaseModel):
    a: Union[TestClassA, TestClassB] = Field(..., description='description...')

... we get the following:

{'definitions': {'TestClassA': {'properties': {},
                                'title': 'TestClassA',
                                'type': 'object'},
                 'TestClassB': {'properties': {},
                                'title': 'TestClassB',
                                'type': 'object'}},
 'properties': {'a': {'anyOf': [{'allOf': [{'$ref': '#/definitions/TestClassA'}]},
                                {'allOf': [{'$ref': '#/definitions/TestClassB'}]}],
                      'description': 'description...',
                      'title': 'A'}},
 'required': ['a'],
 'title': 'TestClassC',
 'type': 'object'}

Take particular note of the additional 'allOf' keys under the 'properties'

We get the same additional 'allOf' keys if we change TestClassC to be the following:

class TestClassC(BaseModel):
    a: Union[TestClassA, TestClassB] = TestClassA()

Right now, we're using a workaround with the schema_extra capability to get rid of these extra allOf keys but this seemed a bit hacky to us. If these allOf keys are the result of a bug, we're happy to provide more test cases that trigger it if that would help. Or, if there's a better way to get rid of it these extra keys that we overlooked, we're happy to help documenting it in any way that we can so others like us don't get confused. Thank you for taking the time to read this issue and for the incredibly useful package!

[samuelcolvin]

Thanks for reporting, I would agree this looks like a bug.

@tiangolo @dmontagu any idea what's going on here?

I'll accept a PR to fix this, but no idea off the top of my head how complicated it would be to fix.

[chriswmackey]

Thank you for the response @samuelcolvin . It's unfortunately going to be at least a couple of months before we can delve into the pydantic source code and attempt a fix. If anyone else is able to tackle it in before then, it would be a huge help.

And, if other users experience this same issue, hopefully they will find this post and see that they can use schema_extra as a workaround for the time being.

[Zac-HD]

https://github.com/samuelcolvin/pydantic/blob/645e5fe6a0c74c95c24410571e9bb804af3eb677/pydantic/schema.py#L693-L696

Given that a length-one-allOf is always a no-op, this branch doesn't actually do anything and skimming the history back to d73aa1b it looks like it has never been necessary (except for the test checking that the result contains allOf, of course).

So it's a good first issue to unconditionally return schema_ref, definitions, nested_models and fix the test 😄

[samuelcolvin]

@tiangolo I guess you first implemented this, any idea why it's there?