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

OpenAPI modelPackage #13995

Closed
Asconius opened this issue Feb 18, 2021 · 6 comments
Closed

OpenAPI modelPackage #13995

Asconius opened this issue Feb 18, 2021 · 6 comments

Comments

@Asconius
Copy link
Contributor

Asconius commented Feb 18, 2021

Overview of the issue

By default, the OpenAPI model (DTOs) is generated in the * .web.api.model package. The ArchTest checks that the services do not use any classes from the web package. It is therefore not possible for service methods to receive and return model classes.

Motivation for or Use Case

Services should be allowed to use the OpenAPI model

Reproduce the error
  1. Insert Components/Schemas into api.yml
  2. Generate OpenAPI files
  3. Use generated DTOs in Service
  4. Start ArchTest
Suggest a Fix

The OpenAPI model should be generated in the package service.dto

From
<modelPackage>*.web.api.model</modelPackage>
To
<modelPackage>*.service.dto</modelPackage>

JHipster Version(s)

v6.10.5

JHipster configuration

OpenAPI generator must be activated when creating the project:
Which other technologies would you like to use? API first development using OpenAPI-generator

@pascalgrimaud
Copy link
Member

can you have a look on this ticket plz @ecostanzi as you're our expert on this part

@atomfrede
Copy link
Member

Should we allow to use generated web(layer) classes inside the service layer? Shouldn't we have proper dto mapping in place?

@ecostanzi
Copy link
Contributor

IMHO it would be hard to strip the DTO imports from the service layer without adding another mapping layer between web and the persistence layer. In my experience being able to manipulate DTOs in the service layer is a good compromise.

I would generate the openAPI DTOs in:
<modelPackage>*.service.api.dto</modelPackage>

@ecostanzi
Copy link
Contributor

I was about to open a PR for this but maybe I rushed to conclusions. On one side I agree with Asconius: since Jhipster DTOs are in the service layer, also OpenAPI DTOs should be generated there. On the other hand, what @atomfrede said made me think that someone could choose to add mapping classes in the web layer or in some other package. It really depends on what developers decide.

Since users are used to having the DTOs in the web layer I prefer to keep the model Package as it is now. For those who want to change it, it's just a one-line change in the pom.xml or gradle file. Open to discussion though.

@Asconius
Copy link
Contributor Author

Asconius commented Mar 4, 2021

Mapping is performed by default in package service.mapper. This can then not be used for the OpenAPI DTO Mapper.
It seems to me as if OpenAPI cannot be integrated into the existing structure.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2021

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs 😃.
Comment or this will be closed in 7 days

ecostanzi added a commit that referenced this issue Apr 11, 2021
@pascalgrimaud pascalgrimaud added this to the 7.1.0 milestone May 28, 2021
@pascalgrimaud pascalgrimaud reopened this Jan 11, 2022
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

5 participants