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

AnonymousSchema file is generated for Enum #1576

Open
Tenischev opened this issue Oct 18, 2023 · 9 comments
Open

AnonymousSchema file is generated for Enum #1576

Tenischev opened this issue Oct 18, 2023 · 9 comments
Labels
bug Something isn't working stale

Comments

@Tenischev
Copy link
Member

Describe the bug

AnonymousSchema class file is generated for Enum instead of Enum with normal name.
Issue is valid for both JavaFileGenerator and JavaGenerator generators.
Issue found during exploration of possibility to use Modelina for model generation in Java spring template, see #asyncapi/java-spring-template#342

How to Reproduce

  1. Use API
  2. Generate with Modelina by:
const modelina = require('@asyncapi/modelina')
const path = require("path");

module.exports = {
    'generate:before': generator => {
        const javaGenerator = new modelina.JavaFileGenerator();

        javaGenerator.generateToFiles(generator.asyncapi, path.resolve(generator.targetDir, 'src/main/java/com/asyncapi/modelina/'), {
            collectionType: "List",
            presets: [
                {
                    preset: modelina.JAVA_COMMON_PRESET,
                    options: {
                        equal: true,
                        hashCode: true,
                        classToString: true
                    }
                }
            ]
        }, true)
    }
};
  1. Find in target directory AnonymousSchema_2 file. This file corresponds to enum property of element from the API:
        command:
          type: string
          enum:
            - on
            - off
          description: Whether to turn on or off the light.
          x-pi: false

Expected behavior

The name of class file should be Command or CommandEnum like the property name, not AnonymousSchema.
Also I do not see a reason to generate this Enum in a separate class file.

@Tenischev Tenischev added the bug Something isn't working label Oct 18, 2023
@Tenischev
Copy link
Member Author

@magicmatatjahu @jonaslagoni @kennethaasan ask for comments

@jonaslagoni
Copy link
Member

The name of class file should be Command or CommandEnum like the property name
Might be better in some cases, but what happens when you encounter two command properties across the AsyncAPI document, with two different entries?

Property names are seriously unstable to use as inferring names, which might work in some cases.

Say we changed to use it, the behavior that Modelina would take is to merge those two models if they have the same ID. Would that be desirable?

But would be happy to access a PR that adaptively changes the naming strategy of the AsyncAPI processor through an option, i.e. we can have multiple variants of this function:

static convertToInternalSchema(
or reuse them where it makes sense.

Would you want to provide a PR for it @Tenischev ?

Also I do not see a reason to generate this Enum in a separate class file.

All individual models are generated into separate files with generateToFiles, you can use generate to mix and match the models as you see fit after the generation process and band them together.

@Tenischev
Copy link
Member Author

@jonaslagoni I want to mention that problem with naming will not occur if not generate dedicated class file for the parameter which is not an object by schema definition. Now, Modelina expand component model described in API and create a problem from this behaviour.
There will be no problem with naming, if "Command or CommandEnum" will be sub-class (property element) for turnOnOffPayload class like it written in the API.
I will have a look what generate could do, but as a developer I want to have tool that simply generates class files for my model, not reading the docs to understand why tool behave strange.
Generation of AnonymousSchema when model doesn't contain any anonymous object is not correct work.

@jonaslagoni
Copy link
Member

There will be no problem with naming, if "Command or CommandEnum" will be sub-class (property element) for turnOnOffPayload class like it written in the API.

Sure would be possible to do, but then if the same enum is referenced in multiple objects, you cannot reuse the instance across them because they will be in different packages 🙂

I will have a look what generate could do, but as a developer I want to have tool that simply generates class files for my model, not reading the docs to understand why tool behave strange.
Generation of AnonymousSchema when model doesn't contain any anonymous object is not correct work.

Each developer will have their own expectation of correct behavior, we will never be able to create a one-fits-all, that's why the tool enables you to mold it as you see it.

@Tenischev
Copy link
Member Author

Tenischev commented Nov 7, 2023

Sure would be possible to do, but then if the same enum is referenced in multiple objects, you cannot reuse the instance across them because they will be in different packages 🙂

This enum is a property of particular object. If API creator would like to reuse enum there all possibilities to make it clear from schema definition by placing it as a separate object.
Why Modelina overthink API and schema design? Why Modelina changes what is written in API?
And OK, maybe "user would like to reuse Enum", but why only Enum? Why not a string or an integer or an array? Let's place every property of each object as a separate class, no need to stop the fantasy.

@jonaslagoni
Copy link
Member

This enum is a property of particular object. If API creator would like to reuse enum there all possibilities to make it clear from schema definition by placing it as a separate object.

Modelina can't know that. We could enable customization through extensions that then control the algorithm slightly. Same with overwriting the behavior with options.

Same with the processing, you are free to create your own processor, in your own repo that explicitly changes how the meta-models are created 🙂 All up to you.

Why Modelina overthink API and schema design? Why Modelina changes what is written in API?

Modelina doesn't care about it being an API. It only changes what is given, to ensure it's valid values in the language it's constrained in. Nowhere in any JSON Schema standard is it defined that the property should act as a name for the enum, it's one interpretation 🙂

And OK, maybe "user would like to reuse Enum", but why only Enum? Why not a string or an integer or an array? Let's place every property of each object as a separate class, no need to stop the fantasy.

If you want that go for it 😛 Modelina will enable you to do it for sure 👍 Just create your own generator, with split being all types, and add a renderer for it 😉

Enum + Classes is just something that every language offers, so it was standardized like that across generators.

Copy link
Contributor

github-actions bot commented Mar 8, 2024

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

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 ❤️

@github-actions github-actions bot added the stale label Mar 8, 2024
@rquinio1A
Copy link
Contributor

rquinio1A commented Apr 25, 2024

Coming from openapi-generator-maven-plugin, I was also suprised by these generated AnonymousSchemaN Java classes that look a bit ugly.

Sorry if I missed this in the discssion above, but what should I do to customize the enum class names ?
It might be worth a section in https://modelina.org/docs/languages/java ?

@github-actions github-actions bot removed the stale label Apr 26, 2024
Copy link
Contributor

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

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 ❤️

@github-actions github-actions bot added the stale label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

No branches or pull requests

3 participants