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

fix(java): solve oneOf using a custom generator APIC-300 #125

Merged
merged 7 commits into from
Mar 4, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Feb 8, 2022

🧭 What and Why

🎟 JIRA Ticket: APIC-300

Because the templating is not enough to generate oneOf for java and maybe other clients, this PR creates a custom generator for java using the command:

yarn openapi-generator-cli meta -o out -n algolia-java -p com.algolia.codegen

And modified it to add the setUseOneOfInterfaces(true) (not accessible otherwise).

Also experimented with adding additionalProperties from Java itself which mean we can add any object we want, like servers.

An issue with openapi-generator-cli breaks custom generator and forces us to use the jar directly but this is getting fixed.

PRs

Follow the stack !

Changes included:

  • Move models to the search package
  • Generate oneOf with custom generator

🧪 Test

@millotp millotp requested a review from a team February 8, 2022 11:20
@millotp millotp self-assigned this Feb 8, 2022
@millotp millotp requested review from eunjae-lee and removed request for a team February 8, 2022 11:20
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

small first pass

GG already!

scripts/generate.sh Outdated Show resolved Hide resolved
openapitools-java.json Outdated Show resolved Hide resolved
generators/README.md Outdated Show resolved Hide resolved
@shortcuts
Copy link
Member

shortcuts commented Feb 8, 2022

Can you use the feat(language) or fix(language) syntax for your PRs? It's not enforced by Semantic yet but we should have something to enforce that later

Same for #102

@millotp millotp changed the title fix: solve oneOf using a custom java generator APIC-300 fix(java): solve oneOf using a custom generator APIC-300 Feb 8, 2022
Base automatically changed from feat/java-cts to main February 9, 2022 09:49
@millotp millotp force-pushed the fix/solve-one-of branch 3 times, most recently from 8dbfaf6 to 1f79eda Compare February 25, 2022 14:25
@netlify
Copy link

netlify bot commented Mar 3, 2022

✔️ Deploy Preview for api-clients-automation canceled.

🔨 Explore the source changes: 06a6efd

🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/6221dc13a2922b0008bd736f

@millotp millotp requested a review from shortcuts March 3, 2022 17:32
@millotp millotp enabled auto-merge (squash) March 3, 2022 17:34
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Only a change required on the JSON files, GG otherwise!!!

config/clients.config.json Outdated Show resolved Hide resolved
shortcuts
shortcuts previously approved these changes Mar 4, 2022
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

GG!!

@millotp millotp requested a review from shortcuts March 4, 2022 09:39
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Looks good!

Side note: we might want to have a small guide on how to handle custom generators, wdyt?

@millotp millotp merged commit dbb1be7 into main Mar 4, 2022
@millotp millotp deleted the fix/solve-one-of branch March 4, 2022 09:42
@millotp
Copy link
Collaborator Author

millotp commented Mar 4, 2022

Good idea, I created a ticket for it.

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

Successfully merging this pull request may close these issues.

2 participants