kbuntrock / openapi-maven-plugin

Generate openapi documentation for SpringMVC or JaxRS/JakartaRS projects.
https://kbuntrock.github.io/openapi-maven-plugin/
MIT License
13 stars 8 forks source link

Model Substitution #98

Closed magx2 closed 2 months ago

magx2 commented 6 months ago

Let say I have object:

public class HumanIdDto {
  private int id;
  // constructors, getters, setters...
}

and endpoint:

@GetRequest('/api/human')
public Human findHuman(@RequestParam("id") HumanIdDto id) {
  //...
}

By default your plugin will create an HumanIdDto object that contains only one field which is technically OK, but it generates "stupid" objects in API.

Can you add some kind of mapping objects to other object, i.e.:

com.acme.HumanIdDto:java.lang.Integer

In this case java class HumanIdDto will be represented as Integer in Open API. Here you have example of plugin for Swagger 2: https://github.com/kongchen/swagger-maven-plugin#sample-model-substitution

kbuntrock commented 6 months ago

This is a very good point. Was thinking about it recently since I cannot know in advance every future java class which need a different mapping (like BigDecimal, LocalDateTime, ...).

Do you want this functionality in the 0.0.17 release? (can take me a few days since I have some plans for my evenings this week. ;) )

magx2 commented 6 months ago

I'm OK with having it in version 18

magx2 commented 6 months ago

BTW this format that I've showed you is only for example. Personally I think it's terrible.

I think that something like this would be better:

<modelSubstitutions>
  <modelSubstitution>
    <from>com.acme.HumanIdDto</from>
    <to>java.lang.Integer</to>
  <modelSubstitution>
</modelSubstitutions>
magx2 commented 6 months ago

Any update on this?

kbuntrock commented 6 months ago

Code wise, now, but I thought about it this week. :) I'm thinking about going a bit further and opening more the generator.

magx2 commented 6 months ago

Can you just release this feature and later add new things? I need this for my work 😉

niedz., 5 lis 2023, 19:17 użytkownik Kévin Buntrock < @.***> napisał:

Code wise, now, but I thought about it this week. :) I'm thinking about going a bit further and opening more the generator.

— Reply to this email directly, view it on GitHub https://github.com/kbuntrock/openapi-maven-plugin/issues/98#issuecomment-1793808657, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWTLHXYTOLKKB52YEOIXQLYC7J5HAVCNFSM6AAAAAA6V2AWAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJTHAYDQNRVG4 . You are receiving this because you authored the thread.Message ID: @.***>

kbuntrock commented 6 months ago

It's more about the time I have to allow to this project. :) But I'll try to work on it tomorrow evening.

kbuntrock commented 6 months ago

Hello @magx2 !

Busy week, lot of work and I'll not be able to work on it today. But I still think about you!

If you really need an emergency release, you can start from your copy of the plugin and modify the OpenApiDataType#fromJavaClass function to directly put your model substitution. Then "mvn install". The snapshot version is now in your local .m2 cache. To share it for the project so your coworker don't have to do anything, you then can create a small maven repository (basically just a directory tree to the plugin pom+jar) and reference it in your pom with this configuration :

<pluginRepositories>
    <pluginRepository>
        <id>repo</id>
        <url>file://${project.basedir}/project_m2</url>
    </pluginRepository>
</pluginRepositories>

I know it's not perfect, but maybe it can help you to wait more peacefully the next release. :)

Wish you a good evening.

magx2 commented 6 months ago

Thanks! It works as a temporary solution, but still I need the fix ;)

magx2 commented 6 months ago

I've added a PR with changes that are required https://github.com/kbuntrock/openapi-maven-plugin/pull/102 .

I know it's not fully done but if you don't have a time you can start from there (FYI there are also changes from the PR #101)

magx2 commented 6 months ago

Any update on this?

kbuntrock commented 6 months ago

Any update on this?

Yes, I was able to work on it this weekend. Still missing in my MR :

magx2 commented 5 months ago

Hey, any update on this?

kbuntrock commented 5 months ago

So sorry Martin, time is flying so fast. A more realistic time of arrival will be Christmas then since I'm planning to take a few days off.

Le mar. 12 déc. 2023, 15:28, Martin @.***> a écrit :

Hey, any update on this?

— Reply to this email directly, view it on GitHub https://github.com/kbuntrock/openapi-maven-plugin/issues/98#issuecomment-1852145411, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADUBIHHOVAXCW32GAAGV7ZTYJBS2RAVCNFSM6AAAAAA6V2AWAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJSGE2DKNBRGE . You are receiving this because you commented.Message ID: @.***>

magx2 commented 5 months ago

FYI: Please make sure that mapping from class to class (not primitive) also works. Previously I did some fast PoC for mapping object to primitive (long) and it was working. Today I had to map from one class to another and it was not working, because I was only changing openApiType without originalType/javaType.

I think it would be good if you would have test for those cases:

  1. OBJECT -> primitive (long, int, string, ...)
  2. OBJECT -> OBJECT
  3. OBJECT -> ARRAY
  4. ARRAY -> OBJECT
  5. OBJECT -> date types
magx2 commented 4 months ago

Hey, do you have a branch with this functionality? Maybe I can build it locally and use it for now.

magx2 commented 4 months ago

Hey, do you have a branch with this functionality? Maybe I can build it locally and use it for now.

?

magx2 commented 4 months ago

Hey, did you had time to fix it?

kbuntrock commented 4 months ago

I've got this, but it is still not finished. Focusing on it for my new year resolutions so you can try it : https://github.com/kbuntrock/openapi-maven-plugin/pull/104/files

As you can see, I'm driving this in a different path than the kongchen plugin.

The openapi-model.yml is the file describing defaults types models (mainly the type / format pairs) It can be extended / partially replaced by anything you want to configure.

Then there is the default "model-association.yml" file, describing how to link a java class to a model. And if it's by "equality" or "assignability". It can also be extended / partially replaced. This should cover your scenarios n°1, 2, 3 and 5 in a very customizable way.

I'm not planning to handle scenario number 4. Do you have use cases for it?

magx2 commented 4 months ago

Awesome! Good that you are back in the game 😁.

I will take a look on your PR tommorow.

About point 4 - I don't have a use case of it. I was just listing all possibilities. After thinking about it in my opinion there is no use case for mapping array to object.

kbuntrock commented 4 months ago

Better to not dig into it then, it would be a genericity nightmare.

magx2 commented 4 months ago

8buwf9

magx2 commented 4 months ago

right?

kbuntrock commented 3 months ago

We can for sure now say it will not be for December. :) Working on it.

kbuntrock commented 3 months ago

Hello @magx2.

I've finally reached a point I'm keen to merge. But I first need to check if it fulfil your needs. Would you mind testing this branch? https://github.com/kbuntrock/openapi-maven-plugin/pull/104

I have not yet wrote the documentation so I write here a draft to help you.

The class mapping is now based on two configuration files : First : the description of component models : https://github.com/kbuntrock/openapi-maven-plugin/pull/104/files#diff-3ffe90d665e945193398ecbf990a4840c2c7e49cff85aba8f7d408d7fa6bc827

Secondly, the mapping between descriptions and scanned class : https://github.com/kbuntrock/openapi-maven-plugin/pull/104/files#diff-86546cf9c0181dd6b47c8c6fc7bb295a81adc3d25ef32f5207de714a52fb4027 A mapping is defined by class equality or class assignability.

The two linked files are the default ones, shipped with the plugin. But one can add an extra mapping file with the option <modelsAssociationsPath>path/to/custom-model-association.yml</modelsAssociationsPath>

And even an extra model file for more complex use cases with the following option : <openapiModelsPath>path/to/custom-model-association.yml</openapiModelsPath>

Please tell me if it check your use case. And don't hesitate if you need an example of usage.

Sorry for the long development time and I wish you a good evening. :)

magx2 commented 3 months ago

I have a question: Why did you introduced 2 new files instead of having the configuration in pom.xml?

magx2 commented 3 months ago

And another question:

I have a class com.foo.Xyz and I want to map it to class com.foo.Abc. How can I do it?

magx2 commented 3 months ago

I've added it to my project and it looks good. I'm not supper happy with extra files (you could map the same thing in pom.xml configuration), but if you prefer it then it's fine

kbuntrock commented 3 months ago

Running out of time tonight. Thank you very much for your time spent testing and reading my PR. Will try to finish reading your comments / questions tomorrow.

kbuntrock commented 3 months ago

Hello @magx2. Here some answers to your questions.

I have a question: Why did you introduced 2 new files instead of having the configuration in pom.xml?

The model file can describe complex types. For this one, I thought describing them in yml is easier. And yml + xml mix terribly. For the mapping file, I did not even think about it, I was focused on the functionality.

But I like your suggestion. As for the freeFields or the default error section, I'll allow to write a definition in json in the xml, or to reference a file. BTW : to be clear, when using file configurations, I advice to not put them in the resource folder since they should not be packed in the application.

I have a class com.foo.Xyz and I want to map it to class com.foo.Abc. How can I do it?

You can't. Imagining the potential use cases, I'm convinced mapping a class to another class is primarily used to map a forgotten/custom class. Here, we can map forgotten class (as you have seen with ZonedDateTime), but we can also create complete custom objects.

But I can be wrong. What about your use case? You really need to remap a class by another class?

magx2 commented 3 months ago

The model file can describe complex types. For this one, I thought describing them in yml is easier. And yml + xml mix terribly. For the mapping file, I did not even think about it, I was focused on the functionality.

I think you are able to do this without mixing yml and xml

But I can be wrong. What about your use case? You really need to remap a class by another class?

It's just not to write open api yml myself but rather leave it to the plugin. I was able do it so you can ignore this request.