asyncapi / modelina

A library for generating typed models based on inputs such as AsyncAPI, OpenAPI, and JSON Schema documents with high customization
https://modelina.org
Apache License 2.0
295 stars 170 forks source link

[BUG] AsyncAPI generator for Java doesn't generate class correctly when there's reference to itself #1885

Open xespirit opened 5 months ago

xespirit commented 5 months ago

Describe the bug.

Modelina Async generator fails to generate Java class correctly in certain scenario: there's self-refence. Example:

{
  "asyncapi": "2.6.0",
  "info": {
    "title": "Case",
    "version": "1.0.0",
    "description": "AsyncAPI schema definition for Case"
  },
  "channels": {},
  "components": {
    "schemas": {
      "CaseSchema": {
        "$id": "CaseSchema",
        "type": "object",
        "properties": {
          "name": {
            "type": "string"
          },
          "label": {
            "type": "string"
          },
          "exceptForCase": {
            "type": "array",
            "items": {
              "type": "object",
              "$ref": "./Case.json#/components/schemas/CaseSchema"
            }
          }
        }
      }
    }
  }
}

Expected behavior

Generated Java class should have private List<CaseSchema> exceptForCase;

instead it generates array of Map<String, Object>:

private List<Map<String, Object>> exceptForCase;

Screenshots

Example:

bug1

Generated class:

bug2

How to Reproduce

See screeshots above

🥦 Browser

None

👀 Have you checked for similar open issues?

🏢 Have you read the Contributing Guidelines?

Are you willing to work on this issue ?

No, someone else can work on it

github-actions[bot] commented 5 months ago

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

jonaslagoni commented 5 months ago

Hey @xespirit, which version are you using?

I am pretty sure this is an old bug from Modelina v2

jonaslagoni commented 5 months ago

In v3 I get the following (https://modelina.org/playground?language=java):

{
  "asyncapi":"2.5.0",
  "info":{
     "title":"Streetlights API",
     "version":"1.0.0",
     "description":"The Smartylighting Streetlights API allows you\nto remotely manage the city lights.\n",
     "license":{
        "name":"Apache 2.0",
        "url":"https://www.apache.org/licenses/LICENSE-2.0"
     }
  },
  "servers":{
     "mosquitto":{
        "url":"mqtt://test.mosquitto.org",
        "protocol":"mqtt"
     }
  },
  "channels":{
     "lightmeasured":{
        "publish":{
           "summary":"Inform about environmental lighting conditions for a particular streetlight.",
           "operationId":"onLightMeasured",
           "message":{
              "name":"LightMeasured",
              "payload":{
                 "$id": "CaseSchema",
                 "type":"object",
                 "properties":{
                    "name":{
                       "type":"string"
                    },
                    "label":{
                       "type":"string"
                    },
                    "exceptForCase":{
                       "type":"array",
                       "items":{
                          "$ref":"#/channels/lightmeasured/publish/message/payload"
                       }
                    }
                 }
              }
           }
        }
     }
  }
}
package asyncapi.models;

import java.util.Map;
public class CaseSchema {
  private String name;
  private String label;
  private CaseSchema[] exceptForCase;
  private Map<String, Object> additionalProperties;

  public String getName() { return this.name; }
  public void setName(String name) { this.name = name; }

  public String getLabel() { return this.label; }
  public void setLabel(String label) { this.label = label; }

  public CaseSchema[] getExceptForCase() { return this.exceptForCase; }
  public void setExceptForCase(CaseSchema[] exceptForCase) { this.exceptForCase = exceptForCase; }

  public Map<String, Object> getAdditionalProperties() { return this.additionalProperties; }
  public void setAdditionalProperties(Map<String, Object> additionalProperties) { this.additionalProperties = additionalProperties; }
}
xespirit commented 5 months ago

Hey @xespirit, which version are you using?

I am pretty sure this is an old bug from Modelina v2

I use 4.0.0-next.16 version of modelina:

"dependencies": {
    "@asyncapi/modelina": "^4.0.0-next.16",
    "ts-node": "^10.9.2",
    "typescript": "4.9.5"
  },
jonaslagoni commented 5 months ago

Ehh. Let me take a look with next 🤔

jonaslagoni commented 5 months ago

@xespirit even with next when I use the following AsyncAPI document it works fine (https://65f02a20166dac9eadcc846e--modelina.netlify.app/playground?language=java):

{
  "asyncapi":"2.5.0",
  "info":{
     "title":"Streetlights API",
     "version":"1.0.0",
     "description":"The Smartylighting Streetlights API allows you\nto remotely manage the city lights.\n",
     "license":{
        "name":"Apache 2.0",
        "url":"https://www.apache.org/licenses/LICENSE-2.0"
     }
  },
  "servers":{
     "mosquitto":{
        "url":"mqtt://test.mosquitto.org",
        "protocol":"mqtt"
     }
  },
  "channels":{
     "lightmeasured":{
        "publish":{
           "summary":"Inform about environmental lighting conditions for a particular streetlight.",
           "operationId":"onLightMeasured",
           "message":{
              "name":"LightMeasured",
              "payload":{
                 "$id": "CaseSchema",
                 "type":"object",
                 "properties":{
                    "name":{
                       "type":"string"
                    },
                    "label":{
                       "type":"string"
                    },
                    "exceptForCase":{
                       "type":"array",
                       "items":{
                          "$ref":"#/channels/lightmeasured/publish/message/payload"
                       }
                    }
                 }
              }
           }
        }
     }
  }
}

Might have something to do with your $ref pointing to the file itself 🤔 Is there a more complex setup in place that might cause this?

xespirit commented 5 months ago

@xespirit even with next when I use the following AsyncAPI document it works fine (https://65f02a20166dac9eadcc846e--modelina.netlify.app/playground?language=java):

{
  "asyncapi":"2.5.0",
  "info":{
     "title":"Streetlights API",
     "version":"1.0.0",
     "description":"The Smartylighting Streetlights API allows you\nto remotely manage the city lights.\n",
     "license":{
        "name":"Apache 2.0",
        "url":"https://www.apache.org/licenses/LICENSE-2.0"
     }
  },
  "servers":{
     "mosquitto":{
        "url":"mqtt://test.mosquitto.org",
        "protocol":"mqtt"
     }
  },
  "channels":{
     "lightmeasured":{
        "publish":{
           "summary":"Inform about environmental lighting conditions for a particular streetlight.",
           "operationId":"onLightMeasured",
           "message":{
              "name":"LightMeasured",
              "payload":{
                 "$id": "CaseSchema",
                 "type":"object",
                 "properties":{
                    "name":{
                       "type":"string"
                    },
                    "label":{
                       "type":"string"
                    },
                    "exceptForCase":{
                       "type":"array",
                       "items":{
                          "$ref":"#/channels/lightmeasured/publish/message/payload"
                       }
                    }
                 }
              }
           }
        }
     }
  }
}

Might have something to do with your $ref pointing to the file itself 🤔 Is there a more complex setup in place that might cause this?

Yes, it has something to do with $ref pointing to itself. There's no any complex setup. I have tried Modelina generator with different json schemas, with different complexity. Everything works well except for case I mentioned above.

Is there any default behaviour of generator so It gneerates Map<String, Object> witjhout even including Java import for Map. My guess it happens when the generator gets confused so it somehow generates the map instead of correct object type.

jonaslagoni commented 5 months ago

Damn, yea thats a bug! https://github.com/asyncapi/modelina/blob/1239c9a03c1cccccd20ef9fd987219ee9d54107f/src/generators/java/JavaConstrainer.ts#L238

We need to add the dependency import like we do for Python here: https://github.com/asyncapi/modelina/blob/1239c9a03c1cccccd20ef9fd987219ee9d54107f/src/generators/python/PythonConstrainer.ts#L20

Do you mind helping to fix this?

xespirit commented 5 months ago

Damn, yea thats a bug!

https://github.com/asyncapi/modelina/blob/1239c9a03c1cccccd20ef9fd987219ee9d54107f/src/generators/java/JavaConstrainer.ts#L238

We need to add the dependency import like we do for Python here:

https://github.com/asyncapi/modelina/blob/1239c9a03c1cccccd20ef9fd987219ee9d54107f/src/generators/python/PythonConstrainer.ts#L20

Do you mind helping to fix this?

I'm sorry but I'm Java/Kotlin Back-end developer, I don't think I have capacity right now to fix Typescript codebase

jonaslagoni commented 5 months ago

No worries, created a good first issue for it: https://github.com/asyncapi/modelina/issues/1892

Yes, it has something to do with $ref pointing to itself. There's no any complex setup. I have tried Modelina generator with different json schemas, with different complexity. Everything works well except for case I mentioned above.

Regarding this problem, I simply cant reproduce your problem, unfortunately... Cause the example you gave generates the correct models.

Can you reproduce it yourself on the playground and post the example? https://65f02a20166dac9eadcc846e--modelina.netlify.app/playground?language=java

AKACHI-4 commented 5 months ago

Hey @jonaslagoni I would love to work on it.

However, Same stands for me as well; I am also unable to generate this problem in playground, I am still wondering why that


image

If the Change is this only, then why only this specific case isn't importing map. It's weird.

{
  "asyncapi": "2.6.0",
  "info": {
    "title": "Case",
    "version": "1.0.0",
    "description": "AsyncAPI schema definition for Case"
  },
  "channels": {},
  "components": {
    "schemas": {
      "CaseSchema": {
        "$id": "CaseSchema",
        "type": "object",
        "properties": {
          "name": {
            "type": "string"
          },
          "label": {
            "type": "string"
          },
          "exceptForCase": {
            "type": "array",
            "items": {
              "type": "object",
              "$ref": "#/components/schemas/CaseSchema"
            }
          }
        }
      }
    }
  }
}
jonaslagoni commented 5 months ago

If the Change is this only, then why only this specific case isn't importing map. It's weird.

It's for every case, where there is a dictionary model 🙂

xespirit commented 5 months ago

@xespirit even with next when I use the following AsyncAPI document it works fine (https://65f02a20166dac9eadcc846e--modelina.netlify.app/playground?language=java):

{
  "asyncapi":"2.5.0",
  "info":{
     "title":"Streetlights API",
     "version":"1.0.0",
     "description":"The Smartylighting Streetlights API allows you\nto remotely manage the city lights.\n",
     "license":{
        "name":"Apache 2.0",
        "url":"https://www.apache.org/licenses/LICENSE-2.0"
     }
  },
  "servers":{
     "mosquitto":{
        "url":"mqtt://test.mosquitto.org",
        "protocol":"mqtt"
     }
  },
  "channels":{
     "lightmeasured":{
        "publish":{
           "summary":"Inform about environmental lighting conditions for a particular streetlight.",
           "operationId":"onLightMeasured",
           "message":{
              "name":"LightMeasured",
              "payload":{
                 "$id": "CaseSchema",
                 "type":"object",
                 "properties":{
                    "name":{
                       "type":"string"
                    },
                    "label":{
                       "type":"string"
                    },
                    "exceptForCase":{
                       "type":"array",
                       "items":{
                          "$ref":"#/channels/lightmeasured/publish/message/payload"
                       }
                    }
                 }
              }
           }
        }
     }
  }
}

Might have something to do with your $ref pointing to the file itself 🤔 Is there a more complex setup in place that might cause this?

I'm providing full sets of files to reproduce the issue:

Case.json DirectPolicy.json Insurance.json Policy.json Schemas.json

The generator is being called from Maven plug-in:

generate ./node_modules/.bin/ts-node ./generate.ts com.test.asyncapi.model model/Schemas.json target/generated-sources/asyncapi/com/test/asyncapi/model

I've attached pictures of generated files, representing the issue below: generated_files

case_schema_bug

@jonaslagoni could you try to reproduce the bug?

As you can see Modelina generator thinks all files are generated successfuly (without exiting with error). But in fact we have:

@JsonProperty("exceptForCase")
@JsonInclude(JsonInclude.Include.NON_NULL)
private List<Map<String, Object>> exceptForCase;

and exceptForCase shouldn't be a list of Map<String, Object>. Expected result is a

private List<CaseSchema> exceptForCase;

jonaslagoni commented 4 months ago

Just pinging that I have not forgotten about this, but have to work on some other fixes that I think will fix this along side it.

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart: