Skip to content
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

feat: Added AsyncAPI v3 internal implementation #501

Merged
merged 2 commits into from Dec 20, 2023

Conversation

ctasada
Copy link
Collaborator

@ctasada ctasada commented Dec 15, 2023

feat: Added AsyncAPI internal implementation

This PR contains a first draft of a Springwolf internal AsyncAPI v3 implementation.

This new module is intended to create AsyncAPI Spec files from Java code, providing different convenient builders

Limitations:

  • The library implementation is still missing multiple Bindings
  • The library implementation works in isolation, work to integrate it with SpringWolf is still pending
  • Support for deserialization is intended in a future iteration
  • Support to validate values is partially implemented
  • While working in the implementation, multiple issues with the existing specification and/or examples were found

Copy link

netlify bot commented Dec 15, 2023

Deploy Preview for springwolf-ui canceled.

Name Link
🔨 Latest commit c16030f
🔍 Latest deploy log https://app.netlify.com/sites/springwolf-ui/deploys/65807bc49e417e0008e16f8d

@ctasada ctasada force-pushed the ctasada/springwolf-asyncapi branch 2 times, most recently from 19d9302 to a33ecc3 Compare December 15, 2023 21:37
@ctasada ctasada marked this pull request as draft December 15, 2023 21:37
@ctasada ctasada force-pushed the ctasada/springwolf-asyncapi branch 2 times, most recently from e6e9368 to 3513e0e Compare December 15, 2023 23:04
@ctasada ctasada mentioned this pull request Dec 15, 2023
2 tasks
@ctasada ctasada force-pushed the ctasada/springwolf-asyncapi branch 3 times, most recently from 507996d to 071f59a Compare December 16, 2023 16:50
Copy link
Member

@timonback timonback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this big contribution and effort to update Springwolf to AsyncApi 3.0!

I like all the classes as well as the test coverage.
Thanks for marking the skipped tests with a reason.

I cam across a couple questions. I do not have an answer on them, so I am interested in the discussion/reasoning:

  1. I notice a lot of classes, which I assume will not directly used by springwolf (i.e. security schema). What is the goal of the library? Support the Springwolf case (or be more generic to support serialization of asyncapi in general)? From our point of view, jAsyncApi wanted to much and therefore wasn't able to support our requirements (type safety, use of Objects, etc).
  2. Has the core in the package io.github.stavshamir.springwolf.asyncapi.core.v3 a special meaning?
  3. The test bindings are yaml, while the test models are json. Does the file format matter? -> I assume this reuses the asyncapi test data directly
  4. What is the basis for the test models? Is the source of truth the spec repo on Github? Link? -> The bindings repo: i.e.: https://github.com/asyncapi/bindings/tree/master/amqp
  5. I wonder whether the bindings are also part of the model package? -> Nevermind, it has its own repo: https://github.com/asyncapi/bindings/tree/master
  6. Regarding the jackson package, I have mixed feelings. However I understand the need of the JsonSerialize annotation on the models. Have you thought about the package name serializer? I can also imagine two interfaces (JsonSerialzer, YamlSerializers) as I like to rather expose those instead of the DefaultAsyncApiSerializer

Although these are a lot of questions, I am very happy with the PR! Hoping this helps improving Springwolf and I am looking forward to the next steps.

settings.gradle Outdated Show resolved Hide resolved
springwolf-asyncapi/build.gradle Show resolved Hide resolved
@ctasada
Copy link
Collaborator Author

ctasada commented Dec 18, 2023

@timonback Regarding your questions

  1. I tried to provide a complete implementation of the spec. I know a lot of those clases are not directly used nowadays, but when trying to pass the test those are required, so I preferred to go the extra mile
  2. Not really, we can remove it. I'm open to suggestions for the package name
  3. The format doesn't matter. The binding examples are copied from the spec and there are done in YAML, so instead of converting them I preferred to keep them as they are provided.
  4. Correct
  5. Correct
  6. No hard attachment. We can easily rename them

@ctasada ctasada marked this pull request as ready for review December 18, 2023 17:04
@timonback timonback merged commit cd88d02 into springwolf:master Dec 20, 2023
16 checks passed
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.

None yet

3 participants