Support for a polymorphic payload

[dabeck81]

Describe the feature request When using a listener with an abstract class (VehicleBase), I want to be able to set all polymorphic classes implementing the abstract class as a payload with "oneOf"(as being done with OpenApi), like so:

components:
  messages:
    io.github.springwolf.examples.kafka.dtos.discriminator.VehicleBase:
      headers:
        $ref: "#/components/schemas/SpringKafkaDefaultHeaders-VehicleBase"
      payload:
        schemaFormat: application/vnd.aai.asyncapi+json;version=3.0.0
        schema:
          oneOf:
            - $ref: "#/components/schemas/io.github.springwolf.examples.kafka.dtos.discriminator.VehicleElectricPayloadDto"
            - $ref: "#/components/schemas/io.github.springwolf.examples.kafka.dtos.discriminator.VehicleGasolinePayloadDto"
      name: io.github.springwolf.examples.kafka.dtos.discriminator.VehicleBase
      title: VehicleBase
      bindings:
        kafka:
          bindingVersion: 0.5.0

Motivation My motivation for this feature request is that we currently have one queue on which all kinds of messages are transmitted. They all have a common header-part describing the data-part. So in my case the Header is the abstract-class that I would define as my payload and all implementing classes would be acceptable on that listener.

Currently, when setting an abstract class as payload a reference to the abstract class is created and the indication that the child-classes are also acceptable is lost. The reason this happens is that the payload always is set as a reference, like you can see here: https://github.com/springwolf/springwolf-core/blob/013a9e146bdb35900e1acc8abed0e85802fd3144/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/scanners/common/AsyncAnnotationScanner.java#L103

If the code would be adapted to set either a reference to a schema-component or either a schema itself, this would resolve a lot of issues, like:

1. Having the need of setting primitive-types as schema-components, like it is done in the code here: https://github.com/springwolf/springwolf-core/blob/013a9e146bdb35900e1acc8abed0e85802fd3144/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/schemas/SwaggerSchemaService.java#L101 You could define the schema directly as payload, like so:

components:
  messages:
    PrimitiveString:
      headers:
        "$ref": "#/components/schemas/SpringRabbitListenerDefaultHeaders"
      payload:
        schemaFormat: application/vnd.aai.asyncapi+json;version=3.0.0
        schema:
          type: string                   --> schema directly instead of reference
      name: PrimitiveString
      title: PrimitiveString
      bindings:
        amqp:
          bindingVersion: 0.3.0

2. It would not be needed anymore to have an envelopping class, like you propose on the website: https://www.springwolf.dev/docs/configuration/documenting-messages#primitive-final-and-external-classes
3. It would resolve an actual bug in the code where the information that the payload is a List is lost. Like can be found in the Kafka-example: https://github.com/springwolf/springwolf-core/blob/013a9e146bdb35900e1acc8abed0e85802fd3144/springwolf-examples/springwolf-kafka-example/src/main/java/io/github/springwolf/examples/kafka/consumers/ExampleConsumer.java#L39 Is translated to just the object, without the list: https://github.com/springwolf/springwolf-core/blob/013a9e146bdb35900e1acc8abed0e85802fd3144/springwolf-examples/springwolf-kafka-example/src/test/resources/asyncapi.yaml#L1106 Ideally this should be mentionned as

payload:
  schemaFormat: application/vnd.aai.asyncapi+json;version=3.0.0
  schema:
    type: array   --> the information that payload is a list
    items:
      "$ref": "#/components/schemas/io.github.springwolf.examples.amqp.dtos.AnotherPayloadDto"

4. It would open up a lot of more customizability, like polymorphism, which is my actual question :-)

Technical details I played a bit around and have a working POC for polymorphism on payload. You can find it here https://github.com/springwolf/springwolf-core/pull/890. The Vehicle-example in Kafka is now working with a oneOf. I am willing to continue the work, but first I had a couple of questions on the correct way to implement:

• As you can see the heavy lifting is happening in the PayloadService, where I added some code to determine if I need to foresee "oneOf" occurences. The code is adopted from springdoc.
• The NamedSchemaObject would ideally be called PayloadSchemaObject with either a referenceObject or schemaObject in it
• It would be nice if the payload-schema could pass through the SwaggerSchemaService, so that Schema's could be adapted with modelConverters. So that in my case the polymorphism conversion happens through a modelconverter instead of in the payloadService. The same way is happening within the spingdoc project for Openapi.
• Also to be able to support primitives, I need a bit of a pointer on exactly how you would be doing this

Let me know on any of your ideas that come to mind for going further with this

[timonback]

Hello again @dabeck81 ,

first of all, I like the idea and agree that the support polymorphics can be improved. The Vehicle example was added recently and I feel there is a ask in the community to expand on it.

I am CCing @SheheryarAamir here, as he recently ask on Discord exactly regarding the oneOf of the abstract class - also based on the comparison with OpenAPI.

Just repeating so that I understand your use case: You have one abstract class and several classes implementing it. You have one listener for the abstract class and expect that the implementing classes are referenced in the schema of the abstract class (oneOf). These implemented classes are part of the components/schemas section of the AsyncAPI document; in addition to the abstract class. (As per previous PR, there is also the @Hidden Object listener)

I assume you use the same/similar payloads with Springwolf and OpenAPI in the same application. On reason that you stumble upon this, is that the generated documents by Springwolf and OpenAPI are different.

Regarding the first set of findings:

1. I am curious how this connects to primitive schemas, but resolving this would be an improvement (inline schemas?)
2. Same as 1.
3. This is a good question, whether this is a (kafka) batch listener, where the actual event is AnotherPayloadDto, or the actual event on the wire is a List. Relates to: https://github.com/springwolf/springwolf-core/blob/5504cd4f68a66b3ccfc70ef246a55a4f1227f9ab/springwolf-core/src/main/java/io/github/springwolf/core/configuration/properties/SpringwolfConfigProperties.java#L257
4. Sound good to me :D

The PR #890 looks promising. I left two small pointers.

Regarding the questions:

1. Yes, PayloadService is a good choice.
2. Ah, I had the same question in the PR. True, NamedSchemaObject is only created within the PayloadService and is only used for headers. Sounds good to me!
3. While Springwolf has a strong dependency on Swagger, we tried to isolate everything related to Swagger. One benefit is that we have better control on the entities and avoid unwanted (Swagger OpenAPI) fields in the final output. Do you have existing ModelConverters code that you want to re-use/share between OpenAPI und Springwolf? One existing Springwolf extension point is SchemasPostProcessor. Can you share a bit more info on how you use the ModelConverters?
4. Primitives as in findings 1.? I could imagine that those can be inlined, however the references still must be detected. Or maybe there is a swagger setting for this? Do you know how OpenAPI solves this? Is this directly related to the polymorphism? It feels like an independent step.

Thank you for the analysis you have put into this already. I tried answering your questions as best as I could and am happy to elaborate more!

[dabeck81]

Hello @timonback ,

The past couple of days I have been working on an implementation for payloads that I think is a bit more flexible.

Following benefits come with this refactoring:

• Primitive-support out of the box from Swagger, no need to be setting it per type.
• Inline schema's are now also possible as payload
• Payloads do not lose information if it is of a collection (bug mentionned under point 3 in Kafka-example)
• A different Message is created when payload is of a single type or collection type (related to point above)
• I removed the TypeToClassConverter for determining the Payload-class, because a Type holds more information of the Type than a Class<?> (also related to point above)
• Resolved a bug in ExampleXmlValueGenerator where a message from the cache needs to change the parent-tag to the one the referencing property has so, for example, message from cache holds example ´123´ and this example is used in a parent class with a different reference, than the example-tag should change to that reference.

Points that I did not refactor and can maybe do in a seperate commit:

• Headers are still created on the "old" way in the swaggerSchemaService. I can do the refactoring when I have time, to make it in line with what I did for the payload. That is the reason you will see duplicate code in that class.
• the UI tests are failing as I have adapted the behaviour of the payload-model. I am not an expert in that field, so any help is appreciated if you would approve my refactoring

Now to clarify what the reason is for this refactoring, is that indeed on our project we are using OpenApi (generated with SpringDoc). We have our own modelConverters and the modelconverters provided by SpringDoc out of the box. It would be nice that all this works together to generate schema's that are the same or quite similar. One very interesting modelconverter is the following, that would add polymorphism out-of-the-box: https://github.com/springdoc/springdoc-openapi/blob/71a0684dc0eea2504f570aad90f5069319dfa061/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/converters/PolymorphicModelConverter.java#L49

I have added by way of test this modelConverter to the project and the Kafka-example generates correctly the OneOf for the VehicleBase-model. The implementing classes are also correctly created with an AllOf referencing the VehicleBase. This is what my premise was for doing the refactoring.

Can you please look at the updated pull-request? Your previous remarks will not be relevant anymore, I hope this is not a problem (PR https://github.com/springwolf/springwolf-core/pull/890).

[timonback]

Hello @dabeck81, the PR bundles a lot of small changes to make something great - sorry for the time it took me some time to review & respond.

I like the changes and also the integration with swagger (ModelConverters & ResolvedSchema). Great to use this as an opportunity to learn more about it. I added a couple comments/questions in the PR.

I did some ui quick-fixes so that it at least compiles. Polishing up the ui is a follow up step.

Aligning this with the headers is a good idea. If possible, I like to prevent the PR from growing even further. So another PR is a good idea.

In case you find it easier to extract certain aspects (i.e. xml bug, renaming of classes), we can get this merged faster & independent throughout the week - as I expect it to be a easy review + pressing the merge button.

[dabeck81]

Hey @timonback ,

Thanks for having looked at my PR. I resolved some little things and left you some ideas that I have based upon your remarks.

The coming weeks I am limited on working as I am taking 3 weeks of holiday, so if you are not too bothered with it, I would leave the PR as is without splitting it in multiple pieces. Once I am back, I can do whatever, split it up, merge it, ...

Greetings,

David

[timonback]

Hey @dabeck81,

Thanks for the update. If you don't mind, I can help with extracting individual parts of the PR, while you become a co-author of those changes. Idea: smaller prs and getting it merged now so that changes/refactorings wont create merge conflicts. Also, probably better to release the xml fix soon. Is that okay for you?

Enjoy your holiday and talk to you soon.

Cheers, Timon

[dabeck81]

Yes of course, you can split it up. Thanks

[dabeck81]

Hey @timonback ,

Any updates about what should happen with the refactoring I did?

Greetings,

David

[timonback]

Hi @dabeck81 , welcome back.

A rebase is a good start, so that it is easier to review for us. It should work automatically as I tried to avoid further changes besides extracting parts.

I like your ideas and where the PR is going and have to revisit your answers next Friday (cc: @sam0r040 ) to better understand how all parts fit together.

FYI, thank you for the inspiration of the model convert to fix an issue with enums.

[timonback]

Update: Rebase is done here: https://github.com/springwolf/springwolf-core/compare/master...timonback:dabeck81?expand=1 (new branch, since I cannot update your branch. Feel free to apply it to your branch and/or copy the commits. For some reason, the git commit owner got rewritten)

EDIT: I added support for extractable classes in the latest commit. It remains open how to proceed with the extraction of Lists -> https://github.com/springwolf/springwolf-core/blob/5504cd4f68a66b3ccfc70ef246a55a4f1227f9ab/springwolf-core/src/main/java/io/github/springwolf/core/configuration/properties/SpringwolfConfigProperties.java#L257

[timonback]

For tracking the current state:

• [x] Support for primitives

• [x] Inline primitive schemas

• [x] Handle polymorphism (?)

• [x] Fix of xml bug

• [x] Handle inlined schemas in ui

• [x] Investigate missing description in schema (or update ui to description from the updated location) -> failing test

• [ ] Create example for inlined schemas

• [x] Cleanup SwaggerSchemaService

  • [x] Headers use the same logic using Type

• [x] Cleanup TypeToClassConverter

  • [x] Migrate TypeToClassConverter tests to PayloadClassExtractor (see TODO)
  • [x] CloudStream plugin uses the new logic
  • [x] Extract classes from nested types like (ConsumerRecord<A, InterestingType>

Stretch Goals:

• [ ] Show polymorphism in ui

@dabeck81 Are you able to edit this comment? If not, I will put it into the issue description, which you can update as well.

---

The PR looks good to me as a first step. A green build is a requirement for merging.

The other items can be addressed in a new PR- although I would like to see the cleanup of the SwaggerSchemaService. I put some trust in you, that you will follow up on it.

[timonback]

@sam0r040 As part of inlining the schema, we stumple upon the special case of missing schema names for primitive types (like name) - just for the xml example generation Code: https://github.com/springwolf/springwolf-core/blob/1746421a7d3d6fddd245ecfb5dfaf0ea5cfdd6b9/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/components/examples/walkers/DefaultSchemaWalker.java#L62

I wonder whether we want to relax the requirement and convert it to an Optional as well, or use a ModelConverter to set a Swagger Schema#name for primitive types.

@dabeck81 Let us know if you have any thoughts on this.

[github-actions[bot]]

The change is staged for release and will be part of the next release.

If you want to try and verify it in your application today, use the latest 1.X.0-SNAPSHOT build as described in our README.md > Testing SNAPSHOT version

Thank you for the report/contribution!

[timonback]

Thank you @dabeck81 for this great contribution and following through the feature in multiple steps!

[dabeck81]

Timon, no problem. It was my pleasure for doing a contribution to this awesome project.

[github-actions[bot]]

The change is available in the latest release. 🎉

Thank you for the report/contribution and making Springwolf better!