Skip to content

SpringDocSealedClassModule overrides Schema annotations #2915

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
Mattias-Sehlstedt opened this issue Feb 23, 2025 · 5 comments · Fixed by #2927
Closed

SpringDocSealedClassModule overrides Schema annotations #2915

Mattias-Sehlstedt opened this issue Feb 23, 2025 · 5 comments · Fixed by #2927

Comments

@Mattias-Sehlstedt
Copy link

The added SpringDocSealedClassModule in the release of version 2.8.5 has made it so that any sealed api-class will have any current polymorphic @Schema annotation override by the module's introspection.

I am wondering if it is intended behavior that the module should always take control of the schema? Or is a better behavior to have the module as a fallback schema provider if the model does not already have an explicit schema defined? I have an example branch here showing how an annotation could look like and what the previous schema generation resulted in. It is the same example as the one shown below in this issue.

TL;DR: would it be logical to introduced so that the module only creates a schema if there is a lack of a @Schema annotation with a OneOf.

An example of a definition could be:

@Schema(name = SuperClass.SCHEMA_NAME,
        discriminatorProperty = "type",
        oneOf = {
                FirstChildClass.class,
                SecondChildClass.class
        },
        discriminatorMapping = {
                @DiscriminatorMapping(value = FirstChildClass.SCHEMA_NAME, schema = FirstChildClass.class),
                @DiscriminatorMapping(value = SecondChildClass.SCHEMA_NAME, schema = SecondChildClass.class)
        }
)
sealed class SuperClass permits FirstChildClass, SecondChildClass {
...
}

Where the schema earlier would be:

{
  "schemas": {
    "Image": {
      "required": [
        "type"
      ],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      }
    },
    "Mail": {
      "required": [
        "type"
      ],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      }
    },
    "SuperClass": {
      "required": [
        "type"
      ],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      },
      "discriminator": {
        "propertyName": "type",
        "mapping": {
          "Image": "#/components/schemas/Image",
          "Mail": "#/components/schemas/Mail"
        }
      },
      "oneOf": [
        {
          "$ref": "#/components/schemas/Image"
        },
        {
          "$ref": "#/components/schemas/Mail"
        }
      ]
    }
  }
}

but now becomes:

{
  "schemas": {
    "Image": {
      "required": [
        "type"
      ],
      "type": "object",
      "allOf": [
        {
          "$ref": "#/components/schemas/SuperClass"
        }
      ]
    },
    "Mail": {
      "required": [
        "type"
      ],
      "type": "object",
      "allOf": [
        {
          "$ref": "#/components/schemas/SuperClass"
        }
      ]
    },
    "SuperClass": {
      "required": [
        "type"
      ],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      },
      "discriminator": {
        "propertyName": "type",
        "mapping": {
          "Image": "#/components/schemas/Image",
          "Mail": "#/components/schemas/Mail"
        }
      },
      "oneOf": [
        {
          "$ref": "#/components/schemas/Image"
        },
        {
          "$ref": "#/components/schemas/Mail"
        }
      ]
    }
  }
}

The issue with this is the recursive SuperClass oneOf Image -> Image allOf SuperClass -> SuperClass oneOf Image -> ....

I am aware that a solution is to disable this new module (by for example overriding the SpringDocProviders and not providing the new module).

But to me it would make more sense if the defined module is rather a fallback for the case when the model does not contain an explicit polymorphic schema annotation already. So I am wondering if such a feature would be accepted if I were to introduce that behavior in a PR (e.g., only attempt to construct a schema for the model if the current @Schema annotation does not contain a OneOf).

@bnasslahsen
Copy link
Collaborator

@sahil-ramagiri,

Can you please check this issue ?

@sahil-ramagiri
Copy link
Contributor

I could replicate the same behavior when JsonSubtypes are used on a non sealed class.

@Schema(name = SuperClass.SCHEMA_NAME,
		discriminatorProperty = "type",
		oneOf = {
				FirstChildClass.class,
				SecondChildClass.class
		},
		discriminatorMapping = {
				@DiscriminatorMapping(value = FirstChildClass.SCHEMA_NAME, schema = FirstChildClass.class),
				@DiscriminatorMapping(value = SecondChildClass.SCHEMA_NAME, schema = SecondChildClass.class)
		}
)
@JsonSubTypes({
		@JsonSubTypes.Type(value = FirstChildClass.class, name = FirstChildClass.SCHEMA_NAME),
		@JsonSubTypes.Type(value = SecondChildClass.class, name = SecondChildClass.SCHEMA_NAME)
})
class SuperClass {
 ...
}

The core problem here seems to be when both @JsonSubTypes and @Schema::oneOf are provoided, it forms a circular reference you see here.

"schemas": {
      "Image": {
        "required": [
          "type"
        ],
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/SuperClass"
          }
        ]
      },

@bnasslahsen any pointers on where to start?

@Mattias-Sehlstedt
Copy link
Author

They are two different issues, but are basically stemming from the same cause (being that two ways of representing oneOf are added at the same time).

The issue that @sahil-ramagiri is highlighting is (as far as I am aware, not 100% sure) due to swagger-core both introspecting their own @Schema annotations, but also Jackson annotations. This issue has existed since before this new springdoc module was introduced, and I always made an explicit choice to not use the Jackson annotations since I wanted the other type of oneOf OpenAPI specification structure.

What the new module does is that it introspects the objects and adds a hierarchy structure that mirrors the one that would be added if Jackson annotations were present.

@sahil-ramagiri
Copy link
Contributor

Here's my observations

Using

@JsonSubTypes({
		@JsonSubTypes.Type(value = FirstChildClass.class, name = FirstChildClass.SCHEMA_NAME),
		@JsonSubTypes.Type(value = SecondChildClass.class, name = SecondChildClass.SCHEMA_NAME)
})
class SuperClass {

Produces

{
  "schemas": {
    "FirstChildClass": {
      "required": ["type"],
      "type": "object",
      "allOf": [
        {
          "$ref": "#/components/schemas/SuperClass"
        }
      ]
    },
    "SecondChildClass": {
      "required": ["type"],
      "type": "object",
      "allOf": [
        {
          "$ref": "#/components/schemas/SuperClass"
        }
      ]
    },
    "SuperClass": {
      "required": ["type"],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      }
    }
  }
}

and using

@Schema(oneOf = {FirstChildClass.class, SecondChildClass.class})
class SuperClass {

produces

{
  "schemas": {
    "FirstChildClass": {
      "required": ["type"],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      }
    },
    "SecondChildClass": {
      "required": ["type"],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      }
    },
    "SuperClass": {
      "required": ["type"],
      "type": "object",
      "properties": {
        "type": {
          "type": "string"
        }
      },
      "oneOf": [
        {
          "$ref": "#/components/schemas/FirstChildClass"
        },
        {
          "$ref": "#/components/schemas/SecondChildClass"
        }
      ]
    }
  }
}

and obviously using both produces a cycle.

I am trying to reason the asymmetry here.

Should we

  1. Try to fix the asymmetry?
  2. Make @Schema take precedence when both @Schema::oneOf and JacksonSubtypes (via annotation or module) are discovered.

@Mattias-Sehlstedt I think the issue is bigger than the SealedClassModule, making it bail out when @Schema annotations are found will just be a temp fix. The downstream users still may extend/provide their own Jackson annotation introspector which will reproduce the same bug.

@sahil-ramagiri
Copy link
Contributor

Anyways, here's my PR that fixes the issue

#2927

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 a pull request may close this issue.

3 participants