Skip to content

Compound models - allOf, anyOf, oneOf support #2299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Compound models - allOf, anyOf, oneOf support #2299

wants to merge 10 commits into from

Conversation

frantuma
Copy link
Member

@frantuma frantuma commented Jul 6, 2017

implements #2277, DON'T MERGE yet, clarify points below and add reorganized tests before merging.

some notes;

  1. added support for Discriminator support in swagger-annotations (see updated Schema annotation, and new annotation DiscriminatorMapping)

  1. users need to use Jackson @JsonSubTypes annotation on "parent" bean, if they want to be able to resolve "children" (e.g. beans with allOf property in @Schema annotation); this is similar to 2.0, but 2.0 allowed also to define a swagger @apimodel(subtypes=..) annotation.

As an example if we define a "parent" bean like:

    @JsonSubTypes({@JsonSubTypes.Type(value = Sub1Bean.class, name = "sub1")})
    static class BaseBean {
        public String type;
        public int a;
        public String b;
    }

and a "child":

    @io.swagger.oas.annotations.media.Schema(description = "Sub1Bean", allOf = {BaseBean.class})
    static class Sub1Bean extends BaseBean {
        public int c;
    }

when we resolve with a code like:

modelResolver = new ModelResolver(new ObjectMapper());
context = new ModelConverterContextImpl(modelResolver);
final Schema baseModel = context.resolve(BaseBean.class);

then both BaseBean and Sub1Bean are resolved

if a @JsonSubTypes is not declared on the parent, resolver won't be able to "know" about Sub1Bean which will not be resolved.


  1. we are resolving a "child" schema (e.g. with allOf) also without needing the "parent" (e.g. the schema/bean referenced in child allOf) to declare the "child" as subtype (i.e. with @JsonSubTypes); in 2.0 handling of @apimodel annotations that was instead mandatory.

e.g with:

    static class BaseBean {
        public String type;
        public int a;
        public String b;
    }

and a "child":

    @io.swagger.oas.annotations.media.Schema(description = "Sub1Bean", allOf = {BaseBean.class})
    static class Sub1Bean extends BaseBean {
        public int c;
    }

when we resolve with a code like:

final Schema baseModel = context.resolve(Sub1Bean.class);

then also BaseBean will be resolved; this was different in 2.0, where a @JsonSubTypes was needed on the parent. See temporarily commented code here


4, atm anyOf (regardless if interfaces) are resolved not as ref model but inline; this is not true for oneOf or allOf. @fehguy @webron what would be the best behaviour here? in 2.0 ref model was used in allOf, possibly we can:

A. resolve anyOf, allOf, and oneOf as referenced models
B. resolve anyOf, allOf, and oneOf as inline models (duplicating possibly definitions)
C. resolve anyOf inline, only if referenced type is interface


  1. in case a @Schema (allOf = ..) annotation is present and/or a @JsonSubTypes is declared on the parent bean, and other properties are declared in the "child" bean, the result JSON will have an allOf field holding the parent schema refs, and will declare its own properties in the properties field.
    This is different from 2.0 behaviour, which "merged" also bean "own" properties into allOf array; I guess the former behaviour is possibly more correct, also possibly previous behaviour was related to the way allOf resolving was handled (parent/child "trigger' methods on ComposedModel).

As an example if we define a "parent" bean like:

    @JsonSubTypes({@JsonSubTypes.Type(value = Sub1Bean.class, name = "sub1")})
    static class BaseBean {
        public String type;
        public int a;
        public String b;
    }

and a "child":

    static class Sub1Bean extends BaseBean {
        public int c;
    }

the serialized JSON/YAML will look like:

{
    "BaseBean": {
        "type": "object",
        "properties": {
            "type": {
                "type": "string"
            },
            "a": {
                "type": "integer",
                "format": "int32",
            },
            "b": {
                "type": "string"
            }
        }
    },
    "Sub1Bean": {
        "allOf": [
            {
                "$ref": "#/components/schemas/BaseBean"
            }        
        ],
        "type": "object",
        "properties": {
            "d": {
                 "type": "integer",
                 "format": "int32"
            }
        }
    }
}


@webron @fehguy is this ok?


  1. in 3.0 there is no currently no support for position field (annotations/model/resolver/spec); is it planned, or should we remove refs in tests, etc?

  1. @webron: idea (ongoing effort) is to reorganize somehow tests in swagger-core module; this would include fix/re-enable/refactor/remove what currently disabled, and optionally write new ones (equivalent to 2.0), idea is more or less the following:
  • DESERIALIZING:

Current: swagger-core/modules/swagger-core/src/test/java/io/swagger/deserialization
swagger-core/modules/swagger-core/src/test/java/io/swagger/oas.models
swagger-core/modules/swagger-core/src/test/java/io/swagger/properties
swagger-core/modules/swagger-core/src/test/java/io/swagger/util
swagger-core/modules/swagger-core/src/test/java/io/swagger

Reorganized: reorganize into swagger-core/modules/swagger-core/src/test/java/io/swagger/deserialization

  • RESOLVING:

Current: swagger-core/modules/swagger-core/src/test/java/io/swagger/jackson
swagger-core/modules/swagger-core/src/test/java/io/swagger

Reorganized: move them all to swagger-core/modules/swagger-core/src/test/java/io/swagger/resolve

  • SERIALIZING:

Current: swagger-core/modules/swagger-core/src/test/java/io/swagger/parameter
swagger-core/modules/swagger-core/src/test/java/io/swagger/properties
swagger-core/modules/swagger-core/src/test/java/io/swagger/util
swagger-core/modules/swagger-core/src/test/java/io/swagger

Reorganized: move them all to swagger-core/modules/swagger-core/src/test/java/io/swagger/serialize

  • FULL CONVERTING:

Current: swagger-core/modules/swagger-core/src/test/java/io/swagger
Reorganized: keep it (renable/refactor,create new)

@frantuma
Copy link
Member Author

closing, related notes/questions in #2312 2312

@frantuma frantuma closed this Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant