Schemas overwrite valid composedSchemas

[eak24]

If there are a couple controllers that look like:

    @JsonSubTypes(@JsonSubTypes.Type(value = SubClass.class))
    @io.swagger.v3.oas.annotations.media.Schema(discriminatorProperty = "type_", discriminatorMapping = {
            @DiscriminatorMapping(value = "SubClass", schema = SubClass.class)})
    public class SuperClass {
        public int hi;
    }

    public class SubClass extends SuperClass{
        public int whatsup;
    }

    public class AnotherController {

        @Path("getSuperClass")
        @io.swagger.v3.oas.annotations.Operation()
        public void getSuperClass(SuperClass superClass) {
        }
    }

    public class Controller {

        @Path("getSubClass")
        @io.swagger.v3.oas.annotations.Operation()
        public void getSubClass(SubClass subClass) {
        }
    }

And then run the Reader.read on this set of controllers, subclass will be a Schema rather than a ComposedSchema. However, if we switch the order of Controller processing by swapping their names (yes - this does switch the order - see Reader, then we do get a ComposedSchema, as expected. Therefore this fails when a composed type is read a final time in a 'non-composed way' - ie, not through the base type. The reason this fails, despite the logic to handle such situations in ModelResolver is that the ModelConverterContextImpl, which keeps track of which types have already been processed gets reconstructed for each read event, losing track of any classes that were resolved by a previous call to the read method. And the only time we 'get lucky' is when the last call was the call that referenced the type through the base type. I have a PR where instead of reconstructing the ModelConverterContextImpl, we just keep it in the ModelResolvers singleton instance. To get the ModelResolver to always correctly locate the given type as being already within the processedTypes I had to change the matching logic to compare canonical names because the two different introspection pathways return two different types, so the only meaningful thing to compare is the names.

[eak24]

@frantuma Is this a reasonable solution to my issue?

[frantuma]

Thanks for reporting and working on it. I am not sure, but I guess this doesn't cover the root "issue" which is resolving a naked child class (no annotations), as e.g. mentioned in #2863
In your case for example, if you just read Controller (with SubClass payload) super class is not resolved.

To obtain the expected behaviour you can add @Schema (allOf = SuperClass.class) to your subclass.

About the PR / processed types identity, it's currently failing many tests, as I believe sharing the context causes quite many side effects; on the other hand it would probably make sense, at lease for performance/resources reasons, it would be great if you could see what the issues causing test failures are, and proceed in case from there

[eak24]

I thought about the 'naked subclass' situation... I actually prefer the way swaggerCore currently works WRT that - we have a structure with several hundred classes, but we only actually need to export a small fraction of that (only a small fraction are exposed)... we don't actually want to expose the deep classes - it would just clutter things up. But if the super is exported, all subs should point to it. I'm looking into the test failures - it is good to hear I am on the right path! Thanks!

[eak24]

@frantuma it looks like the tests are failing because the order of the paths and components was changed due to this code... I'm not sure why that would change the ordering of anything but the components? I think a good solution here would be to just change the data structure implementation of all the maps used within the openApi object to be sorted, so that we guarantee a deterministic order. What do you think of that change?

[frantuma]

I am not sure if that is the issue, also all models classes use ordered Map, so not sure what you mean. Can you share a test where the order is causing the failure?

[frantuma]

@ethan92429 thanks a lot for this, busy times therefore we couldn't review and merge yet, but will do soon

[erikpetzold]

This even happens when generating the schema for return type and then method argument. Argument (post request body) will override the schema from return type.

Here would be my executable example: https://github.com/erikpetzold/swagger-duplicate-schema-generation/blob/main/src/main/java/de/epet/demo/issue2/overriddenschema/SwaggerDemo.java (project contains examples for more issues).

As the solution was discussed 3 years ago and is not yet available, seems this will not be fixed?

[karlvr]

I am also having this problem, pretty much exactly as described in this issue (OP).

I have temporarily worked around it with a change in Components, on the assumption that we prefer a ComposedSchema over any other type... at first I didn't think this was great, and instead I thought that ModelResolver.resolveSubtypes could be modified to not remove "self" from the list of types, etc, but reading the thread above I agree that if we don't ever refer to the superclass then we don't need to represent it in the API... so maybe it is better to decide at the "merge" point, which version of a schema we prefer...

    public Components addSchemas(String key, Schema schemasItem) {
        if (this.schemas == null) {
            this.schemas = new LinkedHashMap<>();
        }
        final Schema existing = this.schemas.get(key);
        if (existing != null) {
            if (schemasItem instanceof ComposedSchema) {
                this.schemas.put(key, schemasItem);
            }
        } else {
            this.schemas.put(key, schemasItem);
        }
        return this;
    }

I do think the current behaviour is faulty so I'd love for us to land on a result that solves it!

I did try the @Schema(allOf = SuperClass.class) but it resulted in references to the annotated class being an inline allOf in the API-spec, as well as the entry in components/schemas being an allOf.