ZenWave360/json-schema-ref-parser-jvm

Code generator issue with nested 'allOf' schema definitions

Closed this issue · 9 comments

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

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..

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

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

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

Hi @ivangsa

The Snapshot works for me. Thanks!

Best regards,
Thorsten

released version 0.8.6

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..

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

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