Skip to content

fix(cts): gen for java #360

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

Merged
merged 10 commits into from
Apr 14, 2022
Merged

fix(cts): gen for java #360

merged 10 commits into from
Apr 14, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Apr 8, 2022

🧭 What and Why

Generate the CTS for all java clients.

Changes included:

  • Add support to type long.
  • Fix import name
  • Add generic type support for List and Map

🧪 Test

CI

@millotp millotp self-assigned this Apr 8, 2022
@netlify
Copy link

netlify bot commented Apr 8, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit 6ed9139
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62582ee0aebc310009037bd1

@algolia-bot
Copy link
Collaborator

algolia-bot commented Apr 8, 2022

✗ The generated branch has been deleted.

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

@@ -0,0 +1 @@
{{#isFreeFormObject}}<String, {{#value.0}}{{#oneOfModel}}{{classname}}{{/oneOfModel}}{{^oneOfModel}}{{objectName}}{{/oneOfModel}}{{> generateGenerics}}{{/value.0}}>{{/isFreeFormObject}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of the missing newline are on purpose, to make the generate code tighter.

List<Pair> acutalQuery = req.getQueryParams();
for (Pair p : acutalQuery) {
assertEquals(expectedQuery.get(p.getName()), p.getValue());
Map<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.searchParams}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not correct according to this ticket but will be fixed in another PR.

@millotp millotp requested review from a team, eunjae-lee and shortcuts and removed request for a team April 12, 2022 15:54
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.

nothing to say! looks really good

@@ -355,13 +355,118 @@ runs:
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/search/**',
'clients/algoliasearch-client-java-2/api/SearchApi.java',
Copy link
Member

Choose a reason for hiding this comment

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

One other solution is to leverage the modelPackage, apiPackage and invokerPackage openapi-generator options so we only have one destination per client (...java-2/clientName), not sure if it works but can make those paths easier to scope

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

anyway this is ought to change soon, those files are not pushed and should not be relevant to the cache, we cannot have any generated files in that list.

Comment on lines +109 to +113
JSONAssert.assertEquals(
"{\"indexName\":\"theIndexName\",\"sourceIndices\":[{\"indexName\":\"testIndex\",\"facets\":[{\"attributes\":\"test\"}],\"generate\":[[\"facetA\",\"facetB\"],[\"facetC\"]]}],\"languages\":[\"french\"],\"exclude\":[\"test\"]}",
req.getBody(),
JSONCompareMode.STRICT_ORDER
);
Copy link
Member

Choose a reason for hiding this comment

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

I think I already asked but isn't it possible to have the unescaped version? D:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope not possible in java, the only way to write string is with double quote

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.

NICE

@millotp millotp merged commit 89ec8fa into feat/all-java Apr 14, 2022
@millotp millotp deleted the feat/java-cts branch April 14, 2022 14:55
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.

3 participants