lerenn / asyncapi-codegen

An AsyncAPI Golang Code generator that generates all Go code from the broker to the application/user. Just plug your application to your favorite message broker!
Apache License 2.0
96 stars 22 forks source link

Generation: `types` generation contains `application` boilerplate. #215

Open wizzardich opened 6 months ago

wizzardich commented 6 months ago

I suppose this is more of an open question / discussion issue. Right now, when asyncapi-codegen -g types is run, there's much more then just types that is being generated:

const AsyncAPIVersion = "0.0.16"

type controller struct {

type ControllerOption func(controller *controller)

type Error struct {
...

Is this intended behaviour? We are actively using type generation from oapi-codegen to create DTOs from specs; the problem with the approach above is two-fold:

  1. We generate a lot of code that remains unused.
  2. If there are multiple services - and thus multiple specs - that we generate types from, these cannot be put into the same package. This causes the naming overlap of the things like controller.

My suggestion would be either to remove generation of that code on -g types calls, or to explicitly add that to the documentation. This way it will be clear that the target generated files cannot be in the same directory.

lerenn commented 6 months ago

This is intended behavior, as this is used by AppController (-g app) and UserController (-g user) but should not be generated twice to avoid naming collision (thus, putting it it types).

I didn't think of the possibility of having multiple service on the same package, firstly because there would be multiple AppController and multiple UserController that would collide with each other, secondly because I did the -g option to separate things (app + types or user + types), not to generate only one part. I'm curious why you're just generating the types without the controller though 🤔

Or I can add it to the documentation, but it wouldn't help your case. Maybe I should duplicate the code and put it with each controller ?

I'll think about it in the next days, but I think the merge with {app/user} controller may be the most suitable solution. Any suggestion is welcomed :)

wizzardich commented 6 months ago

Basically, in our project we are trying to follow the Domain-Driven Design; this means pretty strict separation of domain entities, infrastructure code and DTOs.

We are generating DTOs from specs, and generating Handlers from spec in another part of the package structure. Moreover, atm we do have our own handlers already written; the idea right now is to change types to the types from the spec, and align the spec between producer and consumer services (owned by different people).

It does make sense to me therefore to have the controller types be part of {app/user} controllers ^^

lerenn commented 6 months ago

I makes much more sense to me now ! I'll refactor things to match your case and I'll ask for your confirmation after that :)

amammay commented 4 months ago

just a suggestion while i was checking out this project, if the above template refactor does happen, it might be worth exposing some sort of interface to be able to marshal to and from the types from the brokers message ie.

https://github.com/lerenn/asyncapi-codegen/blob/main/pkg/codegen/generators/v3/templates/message.tmpl#L54

and

https://github.com/lerenn/asyncapi-codegen/blob/main/pkg/codegen/generators/v3/templates/message.tmpl#L166

that way avro/protobuf support could technically be shimmed out

messages:
  userSignedUp:
    payload:
      schemaFormat: 'application/vnd.apache.avro;version=1.9.0'
      x-go-type: UserSignedUpAvro
      x-go-type-import: 
          path: abc.xyz/repo/events/codegen

and then within my codegen package from the example above i could implement the methods for Marshal/Unmarshal as a shim.

That way somebody would be able to "bring" their own types outside of json schema