Adds OpenAPI oneOf for connector properties at runtime#235
Adds OpenAPI oneOf for connector properties at runtime#235NicoPiel wants to merge 4 commits intoOpenIntegrationEngine:mainfrom
Conversation
Adds an OpenAPI/Swagger Schema annotation to the connector properties field to declare concrete property subclasses via oneOf. This enables correct polymorphic schema generation for connector properties, improving API documentation and client code generation. Also imports the Schema annotation required for the new metadata. Signed-off-by: Nico Piel <nico.piel@hotmail.de>
|
Previously missing connectors in a generated |
mgaffigan
left a comment
mgaffigan
left a comment
There was a problem hiding this comment.
I think the intent is good, but this creates a compile-time dependency loop:
graph TD;
com.mirth.connect.connectors.file-->core.mirth.connect.model
core.mirth.connect.model-->com.mirth.connect.connectors.file
core.mirth.connect.model should be happily ignorant of implementations. Any discovery of implementations would need to happen at runtime, but would be instance specific.
|
I could implement an OpenAPI customizer to detect implementations at runtime. |
Replaces a hardcoded list of connector property types with runtime discovery to generate the OpenAPI schema for connector properties. Uses reflection to find concrete ConnectorProperties subclasses and ModelConverters to register their schemas, composing a ConnectorProperties oneOf schema in the OpenAPI components. This keeps API docs in sync with available property implementations and removes the need to manually maintain annotation lists. It also ignores interfaces and abstract classes and preserves a stable ordering for generated schemas. Signed-off-by: Nico Piel <nico.piel@hotmail.de>
|
|
||
| private SortedSet<Class<? extends ConnectorProperties>> findConnectorPropertiesSubtypes() { | ||
| Reflections reflections = new Reflections(new ConfigurationBuilder() | ||
| .forPackages("com.mirth.connect") |
There was a problem hiding this comment.
question: Is it possible to scan the entire classpath?
I don't think you can assume that 3rd-party connectors will use this package name (or the connectors in this repo if we eventually migrate them to other packages.)
There was a problem hiding this comment.
Yeah, I'm pretty sure that's entirely possible. I included that line for testing purposes.
There was a problem hiding this comment.
Can you remove this file from the PR? I think you did not mean for the remaining two changes to still be in place.
Removes an unused Swagger Schema import and tidies up extraneous blank lines in the Connector class. Cleans up imports/formatting to prepare for property annotation changes on the properties-annotations branch. Signed-off-by: Nico Piel <nico.piel@hotmail.de>
Replace package-limited scanning with ClasspathHelper-provided URLs for the Reflections configuration to make discovery of ConnectorProperties subtypes more robust across classloaders and deployment layouts. Signed-off-by: Nico Piel <nico.piel@hotmail.de>
NicoPiel
changed the title
NicoPiel
requested review from
a team,
gibson9583,
jonbartels,
kayyagari,
kpalang,
mgaffigan,
ssrowe and
tonygermano
and removed request for
a team
mgaffigan
left a comment
mgaffigan
left a comment
There was a problem hiding this comment.
Blocking issues:
getSimpleName()is not unique. Deserialization in the current disgusting deserializer is based on full name.- Is it proper to use
ModelConverters.getInstance()directly? It seems odd to me to assume the process will contain exactly one mapping of types to OpenAPI schema. I would assume it is possible to have two differently configured model converters per process.
Non-blocking issues:
3. I do not like relying upon classpath scanning since it pushes us farther away from modern practices like DI and being compatible with AOT. ConnectorProperties subtypes should be exposed by the plugin registry itself.
4. I assume that there are more polymorphic types than just ConnectorProperties - this does not seem structured to accommodate those.
Future work:
5. This does not adjust the deserializer itself (leaving JSON unsupported/nonmatching to the schema, and likely continuing to support RCE vectors). Long term, we should be adding the JsonTypeInfo and dynamic equivalent of JsonSubTypes. ObjectMapper.registerSubtypes seems relevant. (see allowTypes for some nauseous code)
3 participants
Adds an OpenAPI/Swagger Schema annotation to the connector properties field to declare concrete property subclasses via oneOf. This enables correct polymorphic schema generation for connector properties, improving API documentation and client code generation.
Also imports the Schema annotation required for the new metadata.