openapi-core icon indicating copy to clipboard operation
openapi-core copied to clipboard

[Question] Is there a plan to implement discriminator ?

Open sarangsbabu367 opened this issue 4 years ago • 9 comments

  • I have a usecase with validating request by discriminator and it is not supported yet.

sarangsbabu367 avatar Jun 22 '20 15:06 sarangsbabu367

Hi @sarangsbabu367

lack of support for discriminator shouldn't affect on validation process itself. Maybe describe your use case and how discriminator can help you with this.

p1c2u avatar Jun 24 '20 14:06 p1c2u

@p1c2u As you said it is possible to validate a requestBody with a set of schemas using oneOf and anyOf.

  1. Is it the correct way to do it ?
  2. Even though a schemaValidationError is raised, openapi-core is not capable to return proper error. The error is "schema does not valid under any of the given schemas", always.

sarangsbabu367 avatar Jun 24 '20 15:06 sarangsbabu367

My Cat and Dog schema's are quite similar. Only the subschema's are different (CatThings and DogThings). I used a discriminator and mapping to tell them apart.

I did some manual editing to create this minimal example. Forgive my inconstancies and typo's.

'Cat': {
    'properties': {
        'type': {'type': 'string'},
        'value': {'$ref': '#/components/schemas/CatThings'},
    }
}

'Dog': {
    'properties': {
        'type': {'type': 'string'},
        'value': {'$ref': '#/components/schemas/DogThings'},
    }
}

{'content': {
    'items': {
        'type': 'array'
        'oneOf': [
            {'$ref': '#/components/schemas/Cat'},
            {'$ref': '#/components/schemas/Dog'},
        ]},
        'discriminator': {
            'propertyName': 'type'
            'mapping': {
                'cat': '#/components/schemas/Cat',
                'dog': '#/components/schemas/Dog',
            },
        },
    }
}

When I validate, it raises errors:

    generator_class = spectacular_settings.DEFAULT_GENERATOR_CLASS
    generator = generator_class()
    schema = generator.get_schema(request=None, public=True)
    spec = create_spec(schema)
    openapi_response = DjangoOpenAPIResponse(response)
    request_factory = RequestFactory()
    django_request = request_factory.get(url)
    openapi_request = DjangoOpenAPIRequest(django_request)
    validator = ResponseValidator(spec)
    result = validator.validate(openapi_request, openapi_response)
    assert not result.errors, result.errors

schema_errors=(<ValidationError: "{'type': 'cat', 'value': [{'value': {...}}]} is valid under each of {
'$ref': '#/components/schemas/Cat', 'x-scope': ['', '#/components/schemas/SomePage'], 'nullable': False}, 
{'$ref': '#/components/schemas/Dog', 'x-scope': ['', '#/components/schemas/SomePage'], 'nullable': False}">,))], 
data=None, headers={})

OpenAPI core calls the json schema validator. It does not consider the discriminator or mapping. https://github.com/Julian/jsonschema/blob/main/jsonschema/_validators.py#L362-L367

I'm new to OpenAPI specs and validating them, so I might miss something obvious. Maybe my expectations are wrong...

  • Is this a correct use of OneOf, discriminator and mapping?

I hope this gives a clear example of a valid use-case.

allcaps avatar Feb 22 '21 16:02 allcaps

Hi @allcaps Discriminator does not affect on validation process itself. It's just "hinting" for validation tools.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#discriminator-object

Your schema does not validate because your data validates against more than exactly one oneOf schema

To validate against oneOf, the given data must be valid against exactly one of the given subschemas.

https://json-schema.org/understanding-json-schema/reference/combining.html#oneof

If you want to validate against one or more then you should consider using anyOf.

https://json-schema.org/understanding-json-schema/reference/combining.html#anyof

p1c2u avatar Feb 22 '21 17:02 p1c2u

The discriminator field helps in returning more specific errors, i.e. only errors of sub-schemas for which the discriminator field value of the instance is valid are returned. This is useful if you care about providing the most informative error messages to the client. I think it would be nice to have support for discriminators.

@p1c2u Would you accept a PR that adds discriminator support?

sisp avatar May 30 '21 09:05 sisp

Hi @sisp PRs are always welcome

p1c2u avatar Jun 01 '21 08:06 p1c2u

@p1c2u

Hi @allcaps Discriminator does not affect on validation process itself. It's just "hinting" for validation tools.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#discriminator-object

I don't think that is strictly true. For example, if you are using discriminators in the schema for a request body, and the user provides an invalid discriminator value (one that cannot be mapped to a schema), then validation SHOULD fail per the spec. From your link:

Here the discriminator value of dog will map to the schema #/components/schemas/Dog, rather than the default (implicit) value of Dog. If the discriminator value does not match an implicit or explicit mapping, no schema can be determined and validation SHOULD fail.

As another example, consider the following:

MyRequestBodyType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  discriminator:
    propertyName: petType

If validation ignores the petType discriminator property, then it would be possible for a user to provide a payload with all of the properties of the Cat schema, but list the petType as Dog which is nonsensical and it would still pass validation because it matches oneOf the listed schemas (the Cat schema).

For my use-case, I am trying to do API contract testing using the openapi-core library, and validate in my tests requests and responses against the spec.

ewolz3 avatar Jun 02 '21 13:06 ewolz3

For example, if you are using discriminators in the schema for a request body, and the user provides an invalid discriminator value (one that cannot be mapped to a schema), then validation SHOULD fail per the spec

Actually, if you limit your value range by using an enum, you'll get a validation error when you use an invalid discriminator value.

However, I agree with you @ewolz3 on:

If validation ignores the petType discriminator property, then it would be possible for a user to provide a payload with all of the properties of the Cat schema, but list the petType as Dog which is nonsensical and it would still pass validation because it matches oneOf the listed schemas (the Cat schema).

Running into exactly the same situation where I want to use this library for contract testing. Right now additionally to using the validator I manually check that the specified discriminator field contains the expected enum value.

As a workaround I could think of introducing the discriminator field in the sub-type again and specify the single supported value by using an enum with one entry. E.g.:

Animal:
  type: object
  required:
    - type
    properties:
      - type:
  type: string
  enum:
    - Cat
    - Dog
    - Lizard
    ...

Dog:
  allOf:
    - $ref: "#/components/schemas/Animal"
    - type: object
      properties:
        type:
    type: string
    enum:
      - Dog
    ...
...

However, most of typed languages will complain when generating models out of the spec because of the type mismatch on the type field. Though, this is not the case for Python.


What are your thoughts on this @p1c2u? I think that's is a valid point.

kristianmitk avatar Sep 23 '21 13:09 kristianmitk

FWIW I came up with a rudimentary hack to implement support for discriminators:

from openapi_core.unmarshalling.schemas.enums import UnmarshalContext
from openapi_core.unmarshalling.schemas.factories import SchemaUnmarshallersFactory
from openapi_core.validation.request.validators import RequestValidator
from openapi_schema_validator import OAS30Validator


def oneOf(validator, oneOf, instance, schema):
    if discriminator := schema.get("discriminator"):
        mapping = discriminator.get("mapping")
        prop_name = discriminator.get("propertyName")
        value = instance.get(prop_name)
        ref = mapping.get(value)
        oneOf = [x for x in schema["oneOf"] if x["$ref"] == ref]
        schema = schema.copy()
        schema["oneOf"] = oneOf
        del schema["discriminator"]
    _oneOf = OAS30Validator.VALIDATORS["oneOf"]
    yield from _oneOf(validator, oneOf, instance, schema)


class FixedOAS30Validator(OAS30Validator):
    VALIDATORS = {**OAS30Validator.VALIDATORS, "oneOf": oneOf}


class FixedSchemaUnmarshallersFactory(SchemaUnmarshallersFactory):
    def get_validator(self, schema):
        kwargs = {"resolver": self.resolver, "format_checker": self.format_checker}
        if self.context is not None:
            kwargs[self.CONTEXT_VALIDATION[self.context]] = True
        with schema.open() as schema_dict:
            return FixedOAS30Validator(schema_dict, **kwargs)


class FixedRequestValidator(RequestValidator):
    @property
    def schema_unmarshallers_factory(self):
        return FixedSchemaUnmarshallersFactory(
            self.spec.accessor.dereferencer.resolver_manager.resolver,
            self.format_checker,
            self.custom_formatters,
            context=UnmarshalContext.REQUEST,
        )

It doesn't perform as many checks as I'd like, but essentially we select only the matching sub-schema based on the value of the discriminator property on the instance and use only that for validation.

Feel free to try this out as see if it works for you. YMMV.

ngnpope avatar Nov 09 '21 11:11 ngnpope