Java Spring generator 2.0.0 #188
Conversation
Converts previous mapping of AsyncApi 1.0.0 topics to AsyncApi 2.0.0 channels in some basic form for the PublisherService.java template. Bug: asyncapi#134
Discovered the operationId field and replaced the x-service-name from 1.0.0 schemas in the PublisherService template. Updated logic in the MessageHandlerService template to bring in channels instead of topics.
Updates the command line tool from using topics to using channels. Also unified the way that the publishing method names are generated to reduce the chances of mistakes in other templates that reference the publish method.
Fixed generation of the MQTT config file; part of this change also specifies that the template parameter "server" must be specified (similar to the nodejs generator). If the AsyncApi spec file has multiple servers, this allows the generation step to find the right one to use. Also centralizes handler method naming for consistency.
Converted the AmqpConfig.java template to 2.0.0 schema. Added some convenience filters: publishTopics and subscribeTopics and will likely update the other tepmlates to use them.
Updated the application.yml template to support the AsyncApi 2.0.0 schema.
README.md and settings.gradle needed to be updated for the 2.0.0 schema.
Added a post-generation hook that will remove the trailing ":{port}"
from the generated MQTT host configuration parameter in the
application.yml. Also changed the semantics of that host configuration
parameter by removing the prepended "tcp://" from the template and
adding a broker.scheme configuration parameter.
There was a problem hiding this comment.
This is great stuff man. Thanks for taking the time to fix it! I've been trying your code and there are a few things I'd like to give feedback:
- Port variable is assumed in server object. It might not exist and it will fail to generate the code.
- It's automatically publishing messages. For instance, when generating streetlights.yaml, it calls
publisherService.publishReceiveLightMeasurement("Hello World from receiveLightMeasurement"). It would be great to have the code there ready to be called but do not decide when to send the message, as this is really dependent on your business logic. - It's getting the publish/subscribe verbs the other way around. This is tricky when you're working on a code generator (I know 😅) but when you see
publishyour code should actually subscribe and viceversa. Publish and subscribe do not represent what your code does but instead what consumers/clients can do with your code. - Also, when the url is like
mqtt://test.mosquitto.orgit tries to connect totcp://mqtt://test.mosquitto.org. You may want to use a filter like this one: https://github.com/asyncapi/generator/blob/master/templates/nodejs/.filters/all.js#L144.
Hope it helps!
| port: {{server.variables.port.default}} | ||
| scheme: tcp | ||
| host: {{server.url()}} | ||
| port: {{server.variable('port').defaultValue()}} |
There was a problem hiding this comment.
This is failing for me. It's assuming there will be a port variable and it might not exist.
There was a problem hiding this comment.
Thanks for the feedback, I'll have a go over and make sure that the pub/sub is logical! I thought I had followed what was there but perhaps the semantics changed some with channels vs. topics; I should have paid more attention to what the templates were actually outputting! I should be able to get those changes in shortly.
There was a problem hiding this comment.
No worries, it happens to me all the time when working on the code generators.
There was a problem hiding this comment.
Hmm, after reviewing things I'm less comfortable making changes 2 and 3. It seems that the intent of the original author was to have the java-spring generator create client code, so it's written from the perspective of "I downloaded someone else's AsyncApi document and I want to communicate with their API". So, the document indicating that there is a channel that can be published to results in generating code that can indeed publish to the channel, instead of creating code that can receive messages.
I think it's a great idea to have both, similar to OpenAPI: https://openapi-generator.tech/docs/generators.html. In my case, I actually am trying to generate the server side, so I will probably go ahead and create a new template that has the reverse logic as you had suggested. That will be a template that takes the specification and serves the specification, as well as the endpoints it is describing for consumers of the API.
Now, another issue I'm seeing is mostly to do with item 4, but also 1. The AsyncApi specification seems to be unclear in regards to the server.url. The examples given on XXX do not illustrate proper URLs; in fact, I don't see how you could tell whether some of them are supposed to be "relative URLs" or host names. I'm in favor of supporting proper URLs as you have illustrated over schema-less ones; it is unfortunate that the official examples do not follow this (I found asyncapi/spec#274 that raises this concern).
I think what I am going to is treat the example schema I've been using as an example of a relative URL. In my opinion, the generator framework itself should be taking care of passing a proper JS URL entity to the templates. In the case of relative URLs it would require the user to specify a base URL as a command line option. I would be wiling to explore this concept in a branch.
There was a problem hiding this comment.
Sorry for the delay. I agree with you. My recommendation is that you forget about the intent of the original author because this template was always a proof of concept and was never completed. I'd go directly for the "server" implementation first as that's what the other templates are doing. We'll work on the perspective of "this is someone else's AsyncAPI document" later. In regard to the URLs, feel free to treat them as absolute URLs for now meanwhile we fix the other issue, which will probably require a new version of the spec.
|
Hey @nickspacek, can you fix the following spelling mistakes suggested by @RobertDiebels in |
|
I can, although I was hoping to have some feedback from @fmvilas on my wall of text above. I also neglected to re-incorporate the x-service-name as a naming option and opted to just rely on the operation ID, so I'll have to fix that to allow x-service-name as the primary source of the method name when it is present. |
In Java, don't you think it makes more sense if you use |
|
This pull request has been automatically marked as stale because it has not had recent activity 😴 |
|
I'm closing since it's no longer needed. The current Java Spring generator already supports AsyncAPI 2.0.0 and a bunch of more features (kudos to @Tenischev). Thanks anyway, @nickspacek. Feel free to reopen if you think it makes sense. |
I've updated the Java Spring template to work with AsyncApi 2.0.0. I tried to follow the existing formatting but let me know if I wandered or if you notice any inconsistencies.
This should address #134