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

[BUG] [JavaJaxRS] Combining modelNameSuffix and import-mappings #11478

Closed
5 of 6 tasks
mickroll opened this issue Feb 1, 2022 · 7 comments
Closed
5 of 6 tasks

[BUG] [JavaJaxRS] Combining modelNameSuffix and import-mappings #11478

mickroll opened this issue Feb 1, 2022 · 7 comments

Comments

@mickroll
Copy link
Contributor

mickroll commented Feb 1, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

We use <modelNameSuffix>TO</modelNameSuffix> to distinct all our generated models from our internal models and prevent name collissions. In our Spec, we define our Ids as follows (in an extra file id.json):

      "ExampleId": {
        "description": "The example Id.",
        "type": "string",
        "format": "uuid",
        "example": "53b1f9fe-44e1-11f0-1122-336652440000"
      }

The spec then references these id types:

      "ExampleWrapper": {
        "type": "object",
        "required": ["id", "name"],
        "properties": {
          "id": {
            "$ref": "id.json#/components/schemas/ExampleId"
          },
          "name": {
            "type": "string"
          }
        }
      }

To prevent OpenAPI-Generator from generating ExampleId-classes, we define a mapping configuration via <configurationFile>configuration.json</configurationFile>:

{
    "importMappings": {
        "ExampleId": "com.example.model.id.ExampleId"
    }
}

The referenced class ExampleId already exists of course.

All of this worked in 5.3.1 an lead to sources like this:

package com.example.api.generated.model;
[...]
import com.example.model.id.ExampleId;
[...]
public class ExampleWrapperTO {
[...]
  private @Valid ExampleId id;
  private @Valid String name;
[...]
}

With 5.4.0 this is generated:

package com.example.api.generated.model;
[...]
import com.example.api.generated.model.ExampleIdTO;
[...]
public class ExampleWrapperTO {
[...]
  private @Valid ExampleIdTO id;
  private @Valid String name;
[...]

Note: a ExampleIdTO is not generated! (we don't want it to, just a note)

I tracked the changed behaviour to 33bce99#r65722393 but as the PR says, this change is intended. Was it really, given the example above? Or was using modelNamePrefix/modelNameSuffix out of scope for that PR?

Workaround

If i change our configuration.json to the following, valid sources are generated:

{
    "typeMappings": {
        "ExampleIdTO": "ExampleId",
    },
    "importMappings": {
        "ExampleId": "com.example.model.id.ExampleId",
        "ExampleIdTO": "com.example.model.id.ExampleId",
    }
}

But this seems very redundant and partly wrong to me. If i leave out any of the three definitions, it stops working:

  • leave out typeMappings.ExampleIdTO: ExampleWrapperTO uses ExampleIdTO instead of ExampleId
  • leave out importMappings.ExampleId: ExampleWrapperTO uses UUID instead of ExampleId
  • leave out importMappings.ExampleIdTO: ExampleWrapperTO imports com.example.api.generated.model.ExampleIdTO (which doesn't exist), but not com.example.model.id.ExampleId, but uses private @Valid ExampleId id, which cannot be found
openapi-generator version

5.4.0 (worked in 5.3.1)

OpenAPI declaration file content or url

see above

Generation Details

openapi-generator-maven-plugin:5.4.0
with config:

                        <configuration>
                            <inputSpec>${project.basedir}/src/main/resources/META-INF/openapi.json</inputSpec>
                            <configurationFile>${project.basedir}/src/main/openapi/generator/configuration.json</configurationFile>
                            <apiPackage>com.example.api.generated</apiPackage>
                            <modelPackage>com.example.api.generated.model</modelPackage>
                            <modelNameSuffix>TO</modelNameSuffix>
                            <generatorName>jaxrs-spec</generatorName>
                            <!-- <library>quarkus</library> would generate unnecessary Dockerfile etc. -->
                            <configOptions>
                                <interfaceOnly>true</interfaceOnly>
                                <returnResponse>false</returnResponse>
                                <sourceFolder>java</sourceFolder>
                                <useSwaggerAnnotations>false</useSwaggerAnnotations>
                                <generatePom>false</generatePom>
                                <dateLibrary>java8</dateLibrary>
                            </configOptions>
                        </configuration>
Steps to reproduce
Related issues/PRs

#11217

Suggest a fix

Revert #11217 or some of its contents?

mickroll referenced this issue Feb 1, 2022
* remove import mapping logic in model generation, better handle of file, list

* add new files for file, list

* fix some tests

* update tests, doc

* skip file as reserved word in feign client

* add new files

* remove file from reserved word list in spring generator
@anthonymayfield
Copy link

I'm also getting this issue and I'm using this workaround to fix it. I'm setting a languageSpecificPrimitive, these should really be ignored by the modelNameSuffix as I'm already declaring what I want it to be called, if I want the Suffix on it I could just mention it there.

@mickroll
Copy link
Contributor Author

mickroll commented Jul 4, 2022

Behaviour changed again when switching from 6.0.0 to 6.0.1. To achieve the wanted result, the config now has to look like this:

{
    "typeMappings": {
        "ExampleId": "com.example.model.id.ExampleId"
    },
    "schemaMappings": {
        "ExampleId": "needed to trigger typeMappings"
    }
}

@pschichtel
Copy link

this seems to be exactly what I reported in the PR after my tests: #12600 (comment)

@wing328 I guess there really is an issue

@wing328
Copy link
Member

wing328 commented Jul 4, 2022

I'll try to take another look this weekend if someone can provide a minimal/full spec and the settings (e.g. --import-mappings) to reproduce the issue.

@mickroll
Copy link
Contributor Author

mickroll commented Jul 5, 2022

This issue seems to have been fixed by introducing the new schemaMappings configuration option.

@mickroll mickroll closed this as completed Jul 5, 2022
@pschichtel
Copy link

@mickroll will you create a new issue for this: #11478 (comment) ?

@mickroll
Copy link
Contributor Author

mickroll commented Jul 6, 2022

@mickroll will you create a new issue for this: #11478 (comment) ?

No i did not. Feel free to do so, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants