springwolf-core icon indicating copy to clipboard operation
springwolf-core copied to clipboard

Easier control over quotes / always quote string values in YAML?

Open ccudennec-otto opened this issue 1 year ago • 1 comments

Describe the feature request I'm trying to render my asyncapi.yaml in https://studio.asyncapi.com/ but that tool seems to have problems if the values in the YAML are not quoted. As an example: I have an object that defines a date field and defines the example as appointmentDate: 2015-07-20. In that case AsyncAPI studio shows me an error. If I quote the value using double quotes it does not show an error.

I was able to solve the issue by implementing my own AsyncApiSerializerService and defining it as a bean. My implementation will additionally disable the feature Feature.MINIMIZE_QUOTES for YAML.

Motivation I think this is rather an issue of AsyncAPI studio, because the unquoted value is fine according to the JSON schema specification.

But it would be nice to instantly see the result in AsyncAPI studio anyway. Giving easier control over the way the output is rendered would be a good feature anyway IMHO.

Technical details

  • What do you think about turning your ObjectMapper instances into optional beans with specific names? This would simplify the configuration for consumers.
  • What do you think about changing the default in SpringWolf and always quote string values?

Describe alternatives you've considered As explained above, my current solution defines my own implementation of AsyncApiSerializerService with customized behaviour.

I'm happy to get your thoughts and feedback.

Cheers,

Christopher

ccudennec-otto avatar Jun 26 '24 12:06 ccudennec-otto

Hi @ccudennec-otto, thanks a lot for your feedback. It helped us to build a solution for this release in https://github.com/springwolf/springwolf-core/pull/822. Feature.MINIMIZE_QUOTES is now always configured for YAML Examples when using the compatibility mode for AsyncApi Studio (which is ON by default).

Exposing the ObjectMappers sounds like a good idea. Springwolf currently uses multiple ObjectMappers for serialization:

  • https://github.com/springwolf/springwolf-core/blob/12ac039ac5471325bf614ddaed064db2a83c83cb/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/components/examples/walkers/yaml/DefaultExampleYamlValueSerializer.java#L14
  • https://github.com/springwolf/springwolf-core/blob/f512448d5dbb6d4d47b3ab9acac00500fb8bbc60/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/components/examples/walkers/xml/DefaultExampleXmlValueSerializer.java#L17
  • https://github.com/springwolf/springwolf-core/blob/fd9e48c7776a9d14086f7c884f880cdb40c4ae5b/springwolf-core/src/main/java/io/github/springwolf/core/asyncapi/components/examples/walkers/json/ExampleJsonValueGenerator.java#L29
  • https://github.com/springwolf/springwolf-core/blob/f512448d5dbb6d4d47b3ab9acac00500fb8bbc60/springwolf-asyncapi/src/main/java/io/github/springwolf/asyncapi/v3/jackson/DefaultAsyncApiSerializerService.java#L17

You are welcome to allow ObjectMapper customization as a contribution.

sam0r040 avatar Jun 28 '24 14:06 sam0r040

Hi @sam0r040!

I've just tried Spring Wolf 1.5.0 but for me it does not generate different output. The reason is that your code changes did not change the implementation of DefaultAsyncApiSerializerService in #822 or did I miss something? As you've mentioned yourself, all classes that create their own (YAML) ObjectMapper would need to disable the feature.

I can contribute the code changes if you agree that this is missing. 😃

Cheers,

Christopher

ccudennec-otto avatar Jul 29 '24 12:07 ccudennec-otto

Hi @ccudennec-otto, we understood the problem to be located in the example generation, which has also an ObjectMapper. So we fixed that in https://github.com/springwolf/springwolf-core/pull/822 there and added the studio compatiblity mode.

Actually, you can configure the objectMapper of the DefaultAsyncApiSerializerService by calling getYamlObjectMapper and enabling/disabling features.

Of course you are welcome to contribute a code change.

As a side note, lately we have been seeing activity in studio to address the issue there as well: https://github.com/asyncapi/studio/pull/1124

timonback avatar Jul 29 '24 17:07 timonback

I don't know when the code for the example generation is actually used. I generate the AsyncAPI documentation by GETting the YAML response from the AsyncApiController. The AsyncApiSerializerService does not seem to use your changed code.

I try to prepare a PR to illustrate the changes.

As a side note, lately we have been seeing activity in studio to address the issue there as well: https://github.com/asyncapi/studio/pull/1124

Getting the fix in AsyncAPI studio would be even better. 😃

I've also raised an issue on their side to fix the issue a while ago: https://github.com/asyncapi/studio/issues/1119

ccudennec-otto avatar Jul 30 '24 06:07 ccudennec-otto

Validation of type date was fixed in both studio and studio-next. image

aeworxet avatar Jul 30 '24 09:07 aeworxet

Hello, welcome and thank you @aeworxet

I just merged https://github.com/springwolf/springwolf-core/pull/881 so that we got the updated parser-js now.

@ccudennec-otto You can try the snapshot version, probably with springwold asyncapi compatibility mode turn off.

Springwolf generates the asyncapi document, that you http GET, in multiple steps. First, it scans for methods. Then creates the examples (that is the code @sam0r040 changed, as there are json, yaml or xml examples possible, which is independent from the document content type). The examples contain the date strings. Last, you http get the document and use the defaultserializerservice (and its objectmapper).

timonback avatar Jul 30 '24 10:07 timonback

I've just tested the code in Studio and it looks good now! Thanks everyone for your support! 💪

ccudennec-otto avatar Jul 30 '24 11:07 ccudennec-otto

Great, nothing further to do on this issue.

I will re-open the issue for tracking purposes of the next release, and so the maintainers can re-evaluate the compatibility mode introduced in https://github.com/springwolf/springwolf-core/pull/822 for this particular change

timonback avatar Jul 30 '24 12:07 timonback

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!

github-actions[bot] avatar Jul 30 '24 12:07 github-actions[bot]

After removing the compatibility mode in https://github.com/springwolf/springwolf-core/pull/898, this issue can be closed

timonback avatar Aug 04 '24 15:08 timonback

The change is available in the latest release. 🎉

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

github-actions[bot] avatar Aug 30 '24 16:08 github-actions[bot]