Skip to content

[OpenAPI Generator] Enable allow-list for API and Model classes #530

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.

Sign up for GitHub

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

Jump to bottom
Closed
newtork wants to merge 11 commits into from
Closed

[OpenAPI Generator] Enable allow-list for API and Model classes #530

newtork wants to merge 11 commits into from

Conversation

@newtork
Copy link
Contributor

@newtork newtork commented Aug 9, 2024 •
edited
Loading

Please find similar setting names in the original OpenAPI generator.

Usage:

  • In additionalProperties define modelsToGenerate and apisToGenerate. Use <space> or , as delimiter.
<plugin>
  <groupId>com.sap.cloud.sdk.datamodel</groupId>
  <artifactId>openapi-generator-maven-plugin</artifactId>
  <configuration>
    <additionalProperties>
      <apisToGenerate>Sodas,Orders</apisToGenerate>
      <modelsToGenerate>
        Order
        OrderWithTimestamp
        Soda
        SodaWithId
      </modelsToGenerate>
    </additionalProperties>

Note:

  • To enable the generation of file SodasApi.java, the apisToGenerate field should contain Sodas (without suffix),
  • If apisToGenerate is not defined, all APIs are allowed. Same with modelsToGenerate.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@newtork newtork added do not merge Pull request must not be merged please review Request to review a pull request labels Aug 9, 2024
@newtork newtork marked this pull request as draft August 9, 2024 19:23
@newtork newtork mentioned this pull request Aug 14, 2024
Closed
8 tasks
@newtork
Copy link
Contributor Author

newtork commented Sep 23, 2024 •
edited
Loading

Moved stale PR to personal fork.

@newtork newtork closed this Sep 23, 2024
@newtork newtork reopened this Dec 17, 2024
a-d added 2 commits December 17, 2024 11:33
@a-d
Merge remote-tracking branch 'origin/main' into openapi/allowlist-api…
1bbc455 
…s-and-models

# Conflicts:
#	datamodel/openapi/openapi-generator/src/main/java/com/sap/cloud/sdk/datamodel/openapi/generator/GenerationConfigurationConverter.java
@a-d
Format
36955cf
@newtork newtork marked this pull request as ready for review December 17, 2024 10:34
@newtork newtork added please merge Request to merge a pull request and removed do not merge Pull request must not be merged labels Dec 17, 2024
MatKuhr
MatKuhr previously approved these changes Dec 18, 2024
View reviewed changes
Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally lgtm. However, allow-list is not ideal for our purposes. E.g. for orchestration we already have 47 files, and typically we would want to only exclude a few files from generation.

Did you check the options for block-list? Maybe using .openapi-generator-ignore can be made to work?

...or/src/test/resources/DataModelGeneratorIntegrationTest/generate-models/input/sodastore.yaml
version: 1.0.0
description: API for managing sodas in a soda store
paths:
/sodas:
Copy link
Member

@MatKuhr MatKuhr Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor) since there is only one API, this only tests that NotExist doesn't cause the generation to fail..

Copy link
Contributor Author

@newtork newtork Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the implication of this statement

@newtork newtork added please review Request to review a pull request and removed please review Request to review a pull request please merge Request to merge a pull request labels Jan 6, 2025
a-d and others added 2 commits January 29, 2025 09:44
@a-d
Format test setup
a78a10e
@newtork @MatKuhr
Update datamodel/openapi/openapi-generator/src/test/resources/DataMod…
568b31d 
…elGeneratorIntegrationTest/generate-models/input/sodastore.yaml

Co-authored-by: Matthias Kuhr <[email protected]>
newtork
newtork commented Jan 29, 2025
View reviewed changes
Copy link
Contributor Author

@newtork newtork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After realizing the allow/block-listing of model classes leads to non-compileable code, I'm hesitant to introduce this feature. I feel like we need to go back to the drawing board.

@newtork newtork added do not merge Pull request must not be merged and removed please review Request to review a pull request labels Jan 29, 2025
@newtork newtork closed this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MatKuhr MatKuhr MatKuhr left review comments

Assignees

No one assigned

Labels

do not merge Pull request must not be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@newtork @MatKuhr @a-d