chore: set default generator options #385

shortcuts · 2022-04-19T09:37:09Z

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-431

Changes included:

This PR aims at removing hardcoded options by leveraging the custom generators.

Remove bash post-gen scripts logic
- Handled in the custom generator
Remove some custom properties set in the openapitools.json config
- Handled in the custom generator
Remove some models that are not used anymore 🤔
Move removeExistingModel.ts to its own file
Added new pre-gen function: setDefaultGeneratorOptions
- It sets default common options to all generator keys, this could be improved and even moved to custom generators, but it would introduce much more changes

I'm not sure why the deleted files were not deleted before, maybe the codegen does not push them

🧪 Test

CI :D

(Please double check your clients in case I messed up)

netlify · 2022-04-19T09:37:15Z

✅ Deploy Preview for api-clients-automation canceled.

Name	Link
🔨 Latest commit	`09e02be`
🔍 Latest deploy log	https://app.netlify.com/sites/api-clients-automation/deploys/625ee4a1c111e300089cad23

algolia-bot · 2022-04-19T09:38:09Z

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.

shortcuts · 2022-04-19T09:39:15Z

generators/src/main/java/com/algolia/codegen/AlgoliaJavaGenerator.java

+    // set default options of the generator for every clients
+    additionalProperties.put("java8", true);
+    additionalProperties.put("sourceFolder", "algoliasearch-core");
+    setDateLibrary("java8");
+    setSourceFolder("algoliasearch-core");


Here we set to both the additionalProperties and the generator because those variables are used in the templates

shortcuts · 2022-04-19T09:40:23Z

generators/src/main/java/com/algolia/codegen/Utils.java

+    String clientName = "";
+    if (language.equals("javascript")) {
+      // do not capitalize the first part
+      clientName = clientParts[0].toLowerCase();


I've only added the toLowerCase part here

shortcuts · 2022-04-19T09:41:45Z

scripts/pre-gen/setDefaultGeneratorOptions.ts

+    throw new Error(`Generator not found: ${key}`);
+  }
+
+  const hostsOptions = await getHostsOptions({ client, key });


Leveraging the custom gen for this part could be a good next step

millotp

Nice cleanup ! soon we won't need openapitools.json !

millotp · 2022-04-19T11:57:56Z

clients/algoliasearch-client-javascript/packages/algoliasearch-lite/.openapi-generator-ignore

@@ -1,8 +1,7 @@
 # OpenAPI Generator Ignore


is this file useful anymore ? if it's empty we can remove it

Nope, I've kept it as an escape hatch if we want to remove something without touching the generator but it can be removed

I just tried to remove it but it keeps coming back 🤔

I couldn't remove it either, I just added it to .gitignore

yep I guess I'll have to do the same

millotp · 2022-04-19T12:03:46Z

generators/src/main/java/com/algolia/codegen/Utils.java

+      }
+    }
+
+    return clientName;


The client name should still include the + "Api", or + apiNameSuffix here, otherwise the name of the function is false

Or you can have another method that returns that, and createClientName should just add the suffix

I think the name is still relevant, it runs the client name for a certain language, do you have better name ideas?

millotp · 2022-04-19T12:15:31Z

generators/src/main/java/com/algolia/codegen/AlgoliaPhpGenerator.java

+    super.processOpts();
+
+    // set default options of the generator for every clients
+    String client = additionalProperties.get("client").toString();


By moving this code to postProcessOperationsWithModels like in the java generator you can have access to the client name, without adding it to openapitools.json

Even better with this change!! Good point

millotp · 2022-04-19T12:16:56Z

openapitools.json

+        "gitUserId": "algolia",
+        "glob": "specs/bundled/search.yml",
+        "templateDir": "#{cwd}/templates/java/",
+        "generatorName": "algolia-java",


the generator name could be guessed from the language directly in generate.ts, if we assume we always have a custom generator.

I did not made this assumption as, even if it will mostly be the case, first iterations of adding a client could not require it

millotp · 2022-04-19T12:17:51Z

openapitools.json

+        "gitHost": "algolia",
+        "gitUserId": "algolia",
+        "glob": "specs/bundled/search.yml",
+        "templateDir": "#{cwd}/templates/java/",


templateDir can be guessed in the generator directly, as we already now the language.

Yeah actually there's a lot more that can be done on that side, I've went with the minimal implementation as it was to iterate on #386, but I can add the logic to this PR if you want.

…odels`

millotp

Looking good !

millotp · 2022-04-20T08:36:36Z

generators/src/main/java/com/algolia/codegen/AlgoliaJavascriptGenerator.java

+
+    additionalProperties.put("apiName", apiName);
+    additionalProperties.put("capitalizedApiName", Utils.capitalize(apiName));
+    additionalProperties.put("userAgent", Utils.capitalize(spec));


that's a weird name for user agent, and are you sure it's always correct ? I can't remember what pathPrefix is for query-suggestions

It was weird to me too, the pathPrefix is the tag used in the spec, with the case for the current language, so here it's querySuggestions, then we capitalize it to QuerySuggestions 🤔

millotp · 2022-04-20T08:45:05Z

scripts/pre-gen/setDefaultGeneratorOptions.ts

+    gitUserId: 'algolia',
+    glob: `specs/bundled/${client}.yml`,
+    templateDir: `#{cwd}/templates/${language}/`,
+    generatorName: AVAILABLE_CUSTOM_GEN.includes(language)


If this data is accessible here it's also accessible in the generate.ts file where we actually use the generatorName, you can skip the openapitools.json part.
Because we require so much logic now in the custom gen that it's mandatory to have one.

shortcuts added 2 commits April 19, 2022 11:14

chore: set default generator options

f4a8bee

one more deleted file

ab480c9

shortcuts requested review from damcou and millotp April 19, 2022 09:37

shortcuts self-assigned this Apr 19, 2022

shortcuts commented Apr 19, 2022

View reviewed changes

shortcuts added 2 commits April 19, 2022 11:44

update createMatrix

e8bcba8

format gens

2f7a529

millotp reviewed Apr 19, 2022

View reviewed changes

This was referenced Apr 19, 2022

chore: use Client suffix #386

Merged

fix(java): move code to src folder APIC-411 #387

Merged

shortcuts and others added 3 commits April 19, 2022 18:25

remove openapi ignore, move some logic to `postProcessOperationsWithM…

2ddcacb

…odels`

remove unused part

18fd814

Merge branch 'main' into chore/set-default-gen-options

0051573

shortcuts enabled auto-merge (squash) April 19, 2022 16:30

clear cache for real tests

09e02be

shortcuts requested a review from millotp April 20, 2022 08:08

millotp approved these changes Apr 20, 2022

View reviewed changes

shortcuts merged commit 22726d7 into main Apr 20, 2022

shortcuts deleted the chore/set-default-gen-options branch April 20, 2022 08:46

shortcuts added a commit that referenced this pull request Apr 22, 2022

chore: set default generator options (#385)

af5ed37

chore: set default generator options #385

chore: set default generator options #385

Uh oh!

Conversation

shortcuts commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧭 What and Why

Changes included:

🧪 Test

Uh oh!

netlify bot commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for api-clients-automation canceled.

Uh oh!

algolia-bot commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✗ The generated branch has been deleted.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

millotp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

millotp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shortcuts commented Apr 19, 2022 •

edited

Loading

netlify bot commented Apr 19, 2022 •

edited

Loading

algolia-bot commented Apr 19, 2022 •

edited

Loading