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
89 stars 23 forks source link

fix: headers from schema reference support #74

Closed obouchet closed 1 year ago

obouchet commented 1 year ago

Description

While testing the headers support (client), I noticed that it was failing when headers properties were defined as a reference to a schema instead of a plain list of properties. Here is an example of a contract where the generation is not working as expected:

asyncapi: 2.6.0
info:
  title: test
  version: 1.0.0
  description: test
channels:
  channel:
    description: channel
    publish:
      message:
        $ref: '#/components/messages/Message'
components:
  messages:
    Message:
      description: test message
      headers:
        $ref: '#/components/schemas/HeaderSchema'      
      payload:        
        oneOf:
          - $ref: '#/components/schemas/TestSchema'
  schemas:
    HeaderSchema:
      type: object
      description: header
      required:
        - version
        - dateTime
      properties:
        version:
          description: Schema version
          type: string
          example: '1.0.1'
        dateTime:
            description: Date in UTC format "YYYY-MM-DDThh:mm:ss.sZ".
            example: '2023-09-15T20:15:58.0Z'
            type: string
            format: date-time
    TestSchema:
      type: object
      required:
        - obj1
      properties:
        obj1:
          type: object
          required:
            - referenceId
          properties:
            referenceId:
              description: reference ID.
              type: string
              example: "1234567890123456"

There was also an issue supporting the date and date-time format as it was considered as a regular string.

The fix I've made might not cover all test cases. It was sufficient enough to deal with our contract definition and seems to bring no regression on existing use cases.

Since there is no tests structure yet (TBD as I can see), I did not write any tests for these 2 issues.

Let me know if you would like me to write some tests if you already have an idea on how you want to organize your repo to support unit testing of the generation part.

Thank you!

lerenn commented 1 year ago

Damn, thanks for testing out the other types of headers! I'll merge it right away.

Regarding the tests, I'll add a link in the issue for the tests. As I'm adding things, it becomes more and more a priority to add non-regression testing.

Thanks for providing the AsyncAPI file as well!

lerenn commented 1 year ago

Tagged as v0.19.3

obouchet commented 1 year ago

Thank you! Glad I can help a bit. The more we progress with our project, the more I will be testing the generated code so hopefully I will be able to provide more feedback.

Just so you know, in case you were wondering, I'm working for the National Bank of Canada (https://www.nbc.ca/) where I write stream data services using Kafka. All our contracts with internal and external partners are async-api contracts.

Cheers

lerenn commented 1 year ago

Oh nice ! Indeed, I was wondering. Glad to hear that it's used (and battle-tested) in a real environment.

Another insentive to do proper testing: I plan on using the asyncapi documents present in test directory, which now are just used to be sure that generated code compile, to run dockerized tests with supported brokers to be sure that every time we change something, the messages are still received and send in expected ways by each broker (so for now NATS and Kafka).

Once this is done, I will be more confident in doing changes in the codebase.

obouchet commented 1 year ago

+1 for the testing strategy, it seems a good approach to me.

On a side note, I'm just a contractor here at the Bank but I'm advocating for them to do sponsoring to open source projects like yours. I must say it is a tough job to convince them, but there's hope...

lerenn commented 12 months ago

@obouchet That would be awesome, but don't worry, I worked for institutions, and I know how difficult it is to change things :smile:

So no worries if it doesn't work out !