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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 106 additions & 1 deletion .github/actions/cache/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.

'clients/algoliasearch-client-java-2/model/search/**',
'specs/bundled/search.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java recommend client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/RecommendApi.java',
'clients/algoliasearch-client-java-2/model/recommend/**',
'specs/bundled/recommend.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java personalization client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/PersonalizationApi.java',
'clients/algoliasearch-client-java-2/model/personalization/**',
'specs/bundled/personalization.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java analytics client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/AnalyticsApi.java',
'clients/algoliasearch-client-java-2/model/analytics/**',
'specs/bundled/analytics.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java insights client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/Insights.java',
'clients/algoliasearch-client-java-2/model/insights/**',
'specs/bundled/insights.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java abtesting client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/AbtestingApi.java',
'clients/algoliasearch-client-java-2/model/abtesting/**',
'specs/bundled/abtesting.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java query-suggestions client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/QuerySuggestionsApi.java',
'clients/algoliasearch-client-java-2/model/querySuggestions/**',
'specs/bundled/query-suggestions.yml',
'templates/java/**',
'generators/src/**'
)}}

- name: Restore built Java predict client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-java-2/api/PredictApi.java',
'clients/algoliasearch-client-java-2/model/predict/**',
'specs/bundled/predict.yml',
'templates/java/**',
'generators/src/**'
)}}

# Restore PHP clients: used during 'cts' or 'codegen'
- name: Restore built PHP search client
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public Map<String, Object> postProcessSupportingFileData(
// We can put whatever we want in the bundle, and it will be accessible in the
// template
bundle.put("client", createClientName());
bundle.put("import", packageName);
bundle.put("import", createImportName());
bundle.put("hasRegionalHost", hasRegionalHost);
bundle.put("lambda", lambda);

Expand Down Expand Up @@ -252,6 +252,20 @@ private String createClientName() {
return clientName + "Api";
}

private String createImportName() {
if (!language.equals("java")) {
return this.packageName;
}
String[] clientParts = client.split("-");
// do not capitalize the first part
String name = clientParts[0];
for (int i = 1; i < clientParts.length; i++) {
name += Utils.capitalize(clientParts[i]);
}

return name;
}

/**
* override with any special text escaping logic to handle unsafe characters so as to avoid code
* injection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private Map<String, Object> traverseParams(
// free key but only one type
handleMap(paramName, param, testOutput, spec, suffix);
} else {
handlePrimitive(param, testOutput);
handlePrimitive(param, testOutput, spec);
}
return testOutput;
}
Expand All @@ -151,11 +151,15 @@ private Map<String, Object> createDefaultOutput() {
testOutput.put("isObject", false);
testOutput.put("isArray", false);
testOutput.put("isFreeFormObject", false);
testOutput.put("isAnyType", false);
testOutput.put("isString", false);
testOutput.put("isInteger", false);
testOutput.put("isLong", false);
testOutput.put("isDouble", false);
testOutput.put("isBoolean", false);
testOutput.put("isEnum", false);
testOutput.put("isSimpleObject", false);
testOutput.put("oneOfModel", false);

return testOutput;
}
Expand Down Expand Up @@ -219,15 +223,14 @@ private void handleModel(
traverseParams(paramName, param, match, parent, suffix)
);

HashMap<String, String> hashMapOneOfModel = new HashMap();
HashMap<String, String> oneOfModel = new HashMap<>();

hashMapOneOfModel.put("classname", baseType);
hashMapOneOfModel.put(
oneOfModel.put("classname", Utils.capitalize(baseType));
oneOfModel.put(
"name",
getTypeName(match).replace("<", "").replace(">", "")
);

testOutput.put("oneOfModel", hashMapOneOfModel);
testOutput.put("oneOfModel", oneOfModel);

return;
}
Expand Down Expand Up @@ -298,6 +301,10 @@ private void handleObject(
)
);
}
// sometimes it's really just an object
if (testOutput.get("objectName").equals("Object")) {
testOutput.put("isSimpleObject", true);
}

testOutput.put("isFreeFormObject", true);
testOutput.put("value", values);
Expand All @@ -324,7 +331,8 @@ private void handleMap(
IJsonSchemaValidationProperties itemType = items;

// The generator consider a free form object type as an `object`, which
// is wrong in our case, so we infer it.
// is wrong in our case, so we infer it to explore the right path in the traverseParams
// function, but we keep the any type for the CTS.
if (
items == null ||
(items.openApiType.equals("object") && items.isFreeFormObject)
Expand All @@ -333,6 +341,7 @@ private void handleMap(
String paramType = inferDataType(entry.getValue(), maybeMatch, null);

maybeMatch.dataType = paramType;
maybeMatch.isAnyType = true;
itemType = maybeMatch;
}

Expand All @@ -351,9 +360,17 @@ private void handleMap(
testOutput.put("value", values);
}

private void handlePrimitive(Object param, Map<String, Object> testOutput)
throws CTSException {
private void handlePrimitive(
Object param,
Map<String, Object> testOutput,
IJsonSchemaValidationProperties spec
) throws CTSException {
inferDataType(param, null, testOutput);
if (
spec instanceof CodegenParameter && ((CodegenParameter) spec).isAnyType
) {
testOutput.put("isAnyType", true);
}
testOutput.put("value", param);
}

Expand Down Expand Up @@ -402,7 +419,7 @@ private String inferDataType(
return "Integer";
case "Long":
if (spec != null) spec.setIsNumber(true);
if (output != null) output.put("isInteger", true);
if (output != null) output.put("isLong", true);
return "Long";
case "Double":
if (spec != null) spec.setIsNumber(true);
Expand Down
1 change: 1 addition & 0 deletions specs/insights/paths/pushEvents.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ post:
description: A user identifier. Depending if the user is logged-in or not, several strategies can be used from a sessionId to a technical identifier.
timestamp:
type: integer
format: int64
description: Time of the event expressed in milliseconds since the Unix epoch.
queryID:
type: string
Expand Down
9 changes: 3 additions & 6 deletions templates/java/libraries/okhttp-gson/ApiClient.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,9 @@ public class ApiClient {
String jsonStr = JSON.serialize(param);
return jsonStr.substring(1, jsonStr.length() - 1);
} else if (param instanceof Collection) {
StringBuilder b = new StringBuilder();
StringJoiner b = new StringJoiner(",");
for (Object o : (Collection) param) {
if (b.length() > 0) {
b.append(",");
}
b.append(String.valueOf(o));
b.add(String.valueOf(o));
}
return b.toString();
} else {
Expand All @@ -206,7 +203,7 @@ public class ApiClient {
List<Pair> params = new ArrayList<Pair>();

// preconditions
if (name == null || name.isEmpty() || value == null || value instanceof Collection) {
if (name == null || name.isEmpty() || value == null) {
return params;
}

Expand Down
2 changes: 1 addition & 1 deletion templates/java/libraries/okhttp-gson/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public class {{classname}} extends ApiClient {
{{/x-is-custom-request}}
{{#x-is-custom-request}}
for (Map.Entry<String, Object> parameter : parameters.entrySet()) {
queryParams.addAll(this.parameterToPair(parameter.getKey(), parameter.getValue().toString()));
queryParams.addAll(this.parameterToPair(parameter.getKey(), parameter.getValue()));
}
{{/x-is-custom-request}}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{#isFreeFormObject}}<String, {{#value.0}}{{#isAnyType}}Object{{/isAnyType}}{{^isAnyType}}{{#oneOfModel}}{{classname}}{{/oneOfModel}}{{^oneOfModel}}{{objectName}}{{/oneOfModel}}{{> generateGenerics}}{{/isAnyType}}{{/value.0}}>{{/isFreeFormObject}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{{#isArray}}{{#value.0}}<{{#oneOfModel}}{{classname}}{{/oneOfModel}}{{^oneOfModel}}{{objectName}}{{/oneOfModel}}{{> generateGenerics}}>{{/value.0}}{{/isArray}}
{{#isFreeFormObject}}{{^isSimpleObject}}<String, {{#value.0}}{{#isAnyType}}Object{{/isAnyType}}{{^isAnyType}}{{#oneOfModel}}{{classname}}{{/oneOfModel}}{{^oneOfModel}}{{objectName}}{{/oneOfModel}}{{> generateGenerics}}{{/isAnyType}}{{/value.0}}>{{/isSimpleObject}}{{/isFreeFormObject}}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{{#isString}}String {{{key}}}{{suffix}} = "{{{value}}}";{{/isString}}
{{#isInteger}}int {{{key}}}{{suffix}} = {{{value}}};{{/isInteger}}
{{#isLong}}long {{{key}}}{{suffix}} = {{{value}}}L;{{/isLong}}
{{#isDouble}}double {{{key}}}{{suffix}} = {{{value}}};{{/isDouble}}
{{#isBoolean}}boolean {{{key}}}{{suffix}} = {{{value}}};{{/isBoolean}}
{{#isEnum}}{{{objectName}}} {{{key}}}{{suffix}} = {{{objectName}}}.fromValue("{{{value}}}");{{/isEnum}}
{{#isArray}}List {{{key}}}{{suffix}} = new ArrayList(); { {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.add({{{key}}}{{suffix}});{{/value}} }{{/isArray}}
{{#isArray}}List{{> generateGenerics}} {{{key}}}{{suffix}} = new ArrayList<>(); { {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.add({{> maybeConvertOneOf}});{{/value}} }{{/isArray}}
{{#isObject}}{{{objectName}}} {{{key}}}{{suffix}} = new {{{objectName}}}();
{ {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.set{{#lambda.titlecase}}{{{key}}}{{/lambda.titlecase}}({{{key}}}{{suffix}});
{{/value}} }{{/isObject}}{{#isFreeFormObject}}HashMap {{{key}}}{{suffix}} = new HashMap<String, Object>();
{ {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.put("{{{key}}}", {{{key}}}{{suffix}});
{{/value}} }{{/isFreeFormObject}}
{ {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.set{{#lambda.titlecase}}{{{key}}}{{/lambda.titlecase}}({{> maybeConvertOneOf}});
{{/value}} }{{/isObject}}{{#isFreeFormObject}}Map{{> forceMapGenerics}} {{{key}}}{{suffix}} = new HashMap<>();
{ {{#value}}{{> generateParams}}{{parent}}{{parentSuffix}}.put("{{{key}}}", {{> maybeConvertOneOf}});
{{/value}} }{{/isFreeFormObject}}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{#oneOfModel}}{{{classname}}}.of{{{name}}}({{{key}}}{{suffix}}){{/oneOfModel}}{{^oneOfModel}}{{{key}}}{{suffix}}{{/oneOfModel}}
19 changes: 13 additions & 6 deletions tests/CTS/methods/requests/templates/java/requests.mustache
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.algolia.methods.requests;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;

import java.util.*;
Expand All @@ -13,8 +14,8 @@ import com.google.gson.reflect.TypeToken;

import com.algolia.JSON;
import com.algolia.Pair;
import com.algolia.model.search.*;
import com.algolia.search.{{client}};
import com.algolia.model.{{import}}.*;
import com.algolia.api.{{client}};
import com.algolia.utils.echo.*;
import org.skyscreamer.jsonassert.JSONAssert;
import org.skyscreamer.jsonassert.JSONCompareMode;
Expand All @@ -36,7 +37,7 @@ class {{client}}Tests {
{{#parametersWithDataType}}{{> generateParams}}{{/parametersWithDataType}}

EchoResponseInterface req = (EchoResponseInterface) assertDoesNotThrow(() -> {
return client.{{method}}({{#parametersWithDataType}}{{#oneOfModel}}{{{classname}}}.of{{{name}}}({{{key}}}{{suffix}}){{/oneOfModel}}{{^oneOfModel}}{{{key}}}{{suffix}}{{/oneOfModel}}{{^-last}},{{/-last}}{{/parametersWithDataType}});
return client.{{method}}({{#parametersWithDataType}}{{> maybeConvertOneOf}}{{^-last}},{{/-last}}{{/parametersWithDataType}});
});

assertEquals(req.getPath(), "{{{request.path}}}");
Expand All @@ -49,10 +50,16 @@ class {{client}}Tests {
{{/request.data}}

{{#request.searchParams}}
HashMap<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.searchParams}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType());
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.

List<Pair> actualQuery = req.getQueryParams();
for (Pair p : actualQuery) {
assertEquals(expectedQuery.get(p.getName()), p.getValue());
for (Map.Entry<String, String> entry : expectedQuery.entrySet()) {
boolean found = false;
for (Pair p : actualQuery) {
if (p.getName().equals(entry.getKey()) && p.getValue().equals(entry.getValue())) {
found = true;
}
}
assertTrue(found, "Query parameter " + entry.getKey() + " not found in the actual query");
}
{{/request.searchParams}}
}
Expand Down
Loading