RicoSuter / NJsonSchema

JSON Schema reader, generator and validator for .NET
http://NJsonSchema.org
MIT License
1.4k stars 535 forks source link

Support oneOf inheritance for code generators #13

Open RicoSuter opened 8 years ago

RicoSuter commented 8 years ago

https://raw.githubusercontent.com/fge/sample-json-schemas/master/geojson/geometry.json

Branch: one-of-inheritance

RicoSuter commented 5 years ago

See also: https://github.com/RSuter/NJsonSchema/issues/302

LeroyK commented 5 years ago

A combination of oneOf with allOf almost seems to work, except that the client code generator uses the first reference from the list of oneOf references to generate (C#) client code. It should select the base schema if all oneOf references contain a single reference to that base schema in their allOf list.

Example schema:

{
  "openapi": "3.0.1",
  "info": {
    "title": "Pet API",
    "version": "2019-11-06"
  },
  "paths": {
    "/pet": {
      "get": {
        "tags": [
          "Pet"
        ],
        "operationId": "GetPet",
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "oneOf": [
                    {
                      "$ref": "#/components/schemas/Dog"
                    },
                    {
                      "$ref": "#/components/schemas/Bird"
                    }
                  ],
                  "discriminator": {
                    "propertyName": "discriminator"
                  }
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Bird": {
        "required": [
          "wingspan"
        ],
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/Pet"
          }
        ],
        "properties": {
          "wingspan": {
            "type": "integer",
            "format": "int32"
          }
        },
        "additionalProperties": false
      },
      "Dog": {
        "required": [
          "barks"
        ],
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/Pet"
          }
        ],
        "properties": {
          "barks": {
            "type": "boolean"
          }
        },
        "additionalProperties": false
      },
      "Pet": {
        "required": [
          "birthDate",
          "discriminator"
        ],
        "type": "object",
        "properties": {
          "discriminator": {
            "type": "string"
          },
          "name": {
            "type": "string",
            "nullable": true
          },
          "birthDate": {
            "type": "string",
            "format": "date-time"
          }
        },
        "additionalProperties": false,
        "discriminator": {
          "propertyName": "discriminator"
        }
      }
    }
 }
}

Generates in (Dog instead of Pet).

System.Threading.Tasks.Task<Dog> GetPetAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
LeroyK commented 5 years ago

The following code fixes the above.

if (OneOf.Count > 1 &&
    OneOf.Select(x => x.GetActualSchema(checkedSchemas)).ToList() is var oneOfActualSchemas &&
    oneOfActualSchemas.All(x => x.AllOf.Count == 1) &&
    oneOfActualSchemas.Select(x => x.AllOf.FirstOrDefault().GetActualSchema(checkedSchemas)).Distinct().ToList() is var potentialBaseSchemas &&
    potentialBaseSchemas.Count == 1)
{
    checkedSchemas.Add(this);
    return potentialBaseSchemas[0];
}

If you add it here: https://github.com/RicoSuter/NJsonSchema/blob/1a2ce00b3e1e22e78303d1dfeff84b73a2a25392/src/NJsonSchema/JsonSchema.Reference.cs#L99

shadow-cs commented 4 years ago

@RicoSuter question (I just want to be extra clear on the matter): NJsonSchema by-design does not adhere to JSON Schema specification for allOf validation right (see here)? This is intentional since the primary goal of NJsonSchema is to support allOf inheritance as specified by the OpenAPI.

If the above statement is true, may I suggest adding a note to Inheritance wiki page? With a link to this issue. Might help newcomers figuring this stuff out. Thanks.

RicoSuter commented 4 years ago

NJsonSchema by-design does not adhere to JSON Schema specification for allOf validation right

I'd say yes... So ideally we should have an option: https://github.com/RicoSuter/NJsonSchema/issues/302

I think the validators work as expected but if you generate a schema from C# types it will not generate a JSON Schema which works with validation but which works with OpenAPI as this is the primary use case of NJS.

But it makes sense to have an option to choose between OpenAPI inheritance and JSON Schema validation inheritance which would not work with OpenAPI.

If the above statement is true, may I suggest adding a note to Inheritance wiki page?

Yes, please update the wiki if it makes it clearer for other people.

shadow-cs commented 4 years ago

Yes, please update the wiki if it makes it clearer for other people.

Added a note hope it's clear enough...

RicoSuter commented 4 years ago

Ideally we add OpenAPIs inheritance mapping and discriminator to the validator so that this is also supported.. shouldnt be too hard to add

niikoo commented 3 years ago

+1

hideintheclouds commented 3 years ago

Any news regarding this?

wilkovanderveen commented 2 years ago

What is the status of this issue? I am using mulesoft anypoint exchange which generated oas2 specs and converted these to OAS3, but the 'oneOf' issue is still here.

dauthleikr commented 1 year ago

Any movement on this?

Edit: In case anyone also cares about this: I did a dirty "fix" that just uses object (or whatever your AnyType is) as the type instead of the first possible type. To me that is preferable, because now I can just implement a JsonConverter and serialize/deserialize based on some discriminator. Ideally you'd want a base class to be used, but that's not always possible so ¯\(ツ)https://github.com/dauthleikr/NJsonSchema

I don't think this should be merged here, there are way better solutions. This just allows workarounds for the meantime ...

IIphynixII commented 1 year ago

@RicoSuter Are you looking into this at the moment?

rhermans2004 commented 1 year ago

Would really like to see the oneOf way for inheritance added as option to generate schema (from code). Probably extension with unevaluatedProperties is also needed to make this work, as described in JSONSchema. I tried the flattenHierarchy, but that still seems to require such a oneOf construct.