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

Code generator issue with nested 'allOf' schema definitions #7

Closed
tvetter opened this issue Jun 28, 2024 · 9 comments
Closed

Code generator issue with nested 'allOf' schema definitions #7

tvetter opened this issue Jun 28, 2024 · 9 comments
Assignees

Comments

@tvetter
Copy link

tvetter commented Jun 28, 2024

Hi,

I'm using zenwave-sdk-maven-plugin (version 1.6.0) with code generator 'jsonschema2pojo' in order to generate Java classes from an AsyncAPI v3 definition.
There is an an issue with the generated classes when using nested 'allOf' schema definitions as shown in the following sample definition:

asyncapi: 3.0.0
info:
  title: Test-API
  version: '1.0'
channels:
  testTopic:
    messages:
      test:
        name: Test
        payload:
          $ref: '#/components/schemas/Test'
operations:
  publishTest:
    action: send
    summary: publish events
    channel:
      $ref: '#/channels/testTopic'
components:
  schemas:
    Test1:
      allOf:
      - type: object
        properties:
          test1a:
            type: string
      - type: object
        properties:
          test1b:
            type: string
    Test2:
      allOf:
      - type: object
        properties:
          test2a:
            type: string
      - type: object
        properties:
          test2b:
            type: string
    Test:
      type: object
      allOf:
        - $ref: '#/components/schemas/Test1'
        - $ref: '#/components/schemas/Test2'

Expected attributes of class Test: test1a, test1b, test2a, test2b
Actual attributes: test2a, test2b

It looks like during the merge of 'allOf' definitions the properties of the last definition override previously added properties.
See https://github.com/ZenWave360/json-schema-ref-parser-jvm/blob/main/src/main/java/io/zenwave360/jsonrefparser/%24RefParser.java#L141

Thanks and best regards,
Thorsten

@ivangsa ivangsa self-assigned this Jun 28, 2024
@ivangsa
Copy link
Member

ivangsa commented Jun 28, 2024

hi @tvetter

thanks for reporting this and also point to the we the problem is..

I'll be on this next week..

in the time being if you are happy creating a PR on upstream project I will be more than happy releasing it next week..

@ivangsa ivangsa closed this as completed in c03e0e2 Jul 2, 2024
@ivangsa
Copy link
Member

ivangsa commented Jul 2, 2024

hi @tvetter

I've just released version 0.8.5, can you add this dependency to your maven plugin dependencies to confirm it works for your usecase

Take into account that the release may take a few minutes to get into maven central

@tvetter
Copy link
Author

tvetter commented Jul 2, 2024

Hi @ivangsa

Thanks for the quick fix. It works for the provided sample definition.

However there is still an issue with the following definition:

asyncapi: 3.0.0
info:
  title: Test Event-API
  version: '1.0'
  description: Test
channels:
  testTopic:
    messages:
      test:
        name: Test
        payload:
          $ref: '#/components/schemas/Test'
operations:
  publishTest:
    action: send
    summary: publish events
    channel:
      $ref: '#/channels/testTopic'
    messages:
    - $ref: '#/channels/testTopic/messages/test'
components:
  schemas:
    Test1:
      allOf:
      - type: object
        properties:
          test1a:
            type: string
      - $ref: '#/components/schemas/Test1b'
    Test2:
      allOf:
      - type: object
        properties:
          test2a:
            type: string
      - type: object
        properties:
          test2b:
            type: string
    Test1b:
      allOf:
      - type: object
        properties:
          test1b:
            type: string
      - type: object
        properties:
          test1c:
            type: string
    Test:
      type: object
      allOf:
        - $ref: '#/components/schemas/Test1'
        - $ref: '#/components/schemas/Test2'

Expected attributes: test1a, test1b, test1c, test2a, test2b
Actual attributes: test1b, test1c

I wasn't able to analyze the code yet. I will let you know in case I have some pointers.

Thanks and best regards,
Thorsten

@ivangsa
Copy link
Member

ivangsa commented Jul 3, 2024

hi @tvetter

I've uploades a (hopefully) generic fix for this..

Can you download and build from source in order to test this snapshot before making a release

Thanks

@tvetter
Copy link
Author

tvetter commented Jul 3, 2024

Hi @ivangsa

The Snapshot works for me. Thanks!

Best regards,
Thorsten

@ivangsa
Copy link
Member

ivangsa commented Jul 3, 2024

released version 0.8.6

@ivangsa
Copy link
Member

ivangsa commented Jul 4, 2024

hey @tvetter

thanks for taking the time to report this, it helps make the library better..

I have one question if you don't mind:

  • Are you also using zenwave's spring-cloud-streams3 generator for asyncapi ?
    • If not, is there anything missing for you to consider it?
    • If yes, what do you think about spring-modulith support for event brokers? because I'm thinking to create an integration that will benefit for all the funcionality that cames with spring-modulith, like the transactional outbox and more..

@tvetter
Copy link
Author

tvetter commented Jul 29, 2024

Hey @ivangsa

I didn't use spring-cloud-streams3 generator yet. Right now I'm evaluating the usability of AsyncAPI in my company. Being able to generate payload DTOs from AsyncAPI schema definitions is one of our main requirements. Zenwave looks like a perfect fit for that. Generating interfaces may become interesting for the developers once we actually use AsyncAPI. But we are not there yet.

Best regards,
Thorsten

@tvetter
Copy link
Author

tvetter commented Aug 13, 2024

Hi @ivangsa

there is one more minor issue with allOf definitions. The generated class name is 'null' when the message payload references an allOf schema and no explicit message name is defined as shown in the following definition:

asyncapi: 3.0.0
info:
  title: Test-API
  version: '1.0'
channels:
  testTopic:
    messages:
      test:
        payload:
          $ref: '#/components/schemas/Test'
operations:
  publishTest:
    action: send
    summary: publish events
    channel:
      $ref: '#/channels/testTopic'
components:
  schemas:
    Test:
      allOf:
      - type: object
        properties:
          test1:
            type: string
      - type: object
        properties:
          test2:
            type: string

With a provided message name that name is used as class name. As the message name is optional the schema name should be used in absence of the message name. As I said that is a minor issue. Just wanted to let you know.

Working definition:

asyncapi: 3.0.0
info:
  title: Test-API
  version: '1.0'
channels:
  testTopic:
    messages:
      test:
        name: Test
        payload:
          $ref: '#/components/schemas/Test'
operations:
  publishTest:
    action: send
    summary: publish events
    channel:
      $ref: '#/channels/testTopic'
components:
  schemas:
    Test:
      allOf:
      - type: object
        properties:
          test1:
            type: string
      - type: object
        properties:
          test2:
            type: string

Best regards,
Thorsten

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

No branches or pull requests

2 participants