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): escape oneOf naming #268

Merged
merged 2 commits into from
Mar 21, 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
1 change: 1 addition & 0 deletions .github/actions/cache/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ runs:
hashFiles(
'clients/algoliasearch-client-java-2/**',
'!clients/algoliasearch-client-java-2/target',
'templates/java/**',
'specs/bundled/search.yml'
)}}

Expand Down
1 change: 1 addition & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ jobs:
hashFiles(
format('{0}/**', matrix.client.folder),
format('!{0}/target', matrix.client.folder),
'templates/java/**',
format('specs/bundled/{0}.yml', matrix.client.name)
)}}

Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ dist
.openapi-generator

tests/output/*/.openapi-generator-ignore

generators/bin
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is generating this bin ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The generators when built locally without the docker image, I'm not sure why it did not happened before

Copy link
Collaborator

Choose a reason for hiding this comment

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

They are built by the scripts inside the docker image, and should only create a .gradle and build folder, do you have something else installed on your IDE ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I don't even have a Java extension on my vscode.

Do you want me to remove it from the gitignore?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No if you have it something is generating it, I just wanted to understand what

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do more tests in #227 later, will let you know if I know what is the issue

Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,19 @@ public Map<String, Object> postProcessAllModels(Map<String, Object> objs) {
CodegenModel model = ((Map<String, List<Map<String, CodegenModel>>>) modelContainer).get("models").get(0)
.get("model");
if (!model.oneOf.isEmpty()) {
List<HashMap<String, String>> listOneOf = new ArrayList();

for (String iterateModel : model.oneOf) {
HashMap<String, String> hashMapOneOf = new HashMap();

hashMapOneOf.put("type", iterateModel);
hashMapOneOf.put("name", iterateModel.replace("<", "").replace(">", ""));

listOneOf.add(hashMapOneOf);
}

model.vendorExtensions.put("x-is-one-of-interface", true);
model.vendorExtensions.put("x-is-one-of-list", listOneOf);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,14 @@ private void handleModel(String paramName, Object param, Map<String, Object> tes
IJsonSchemaValidationProperties match = findMatchingOneOf(param, model);
testOutput.clear();
testOutput.putAll(traverseParams(paramName, param, match, parent, suffix));
testOutput.put("oneOfModel", baseType);

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

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

testOutput.put("oneOfModel", hashMapOneOfModel);

return;
}

Expand Down
20 changes: 10 additions & 10 deletions templates/java/oneof_interface.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import java.io.IOException;
@JsonAdapter({{classname}}.Adapter.class)
{{>additionalModelTypeAnnotations}}
public abstract class {{classname}} implements CompoundType {
{{#oneOf}}
public static {{classname}} of({{.}} inside) {
return new {{classname}}{{.}}(inside);
{{#vendorExtensions.x-is-one-of-list}}
public static {{classname}} of{{name}}({{{type}}} inside) {
return new {{classname}}{{name}}(inside);
}

{{/oneOf}}
{{/vendorExtensions.x-is-one-of-list}}
public abstract Object getInsideValue();

public static class Adapter extends TypeAdapter<{{classname}}> {
Expand All @@ -37,18 +37,18 @@ public abstract class {{classname}} implements CompoundType {
}
}

{{#oneOf}}
{{#vendorExtensions.x-is-one-of-list}}
@JsonAdapter({{classname}}.Adapter.class)
class {{classname}}{{.}} extends {{classname}} {
private final {{.}} insideValue;
class {{classname}}{{name}} extends {{classname}} {
private final {{{type}}} insideValue;

{{classname}}{{.}}({{.}} insideValue) {
{{classname}}{{name}}({{{type}}} insideValue) {
this.insideValue = insideValue;
}

@Override
public {{.}} getInsideValue() {
public {{{type}}} getInsideValue() {
return insideValue;
}
}
{{/oneOf}}
{{/vendorExtensions.x-is-one-of-list}}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class {{client}}Tests {
{{#parametersWithDataType}}{{> generateParams}}{{/parametersWithDataType}}

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

assertEquals(req.getPath(), "{{{request.path}}}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,10 @@ void deleteByTest0() {
}

EchoResponseInterface req = (EchoResponseInterface) assertDoesNotThrow(() -> {
return client.deleteBy(indexName0, SearchParams.of(searchParams0));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised those are the only changes to the test, I think there is something more, I will check.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is definitely a missing part with the CTS I think, I still can't make #227 work :(

return client.deleteBy(
indexName0,
SearchParams.ofSearchParamsObject(searchParams0)
);
}
);

Expand Down Expand Up @@ -1722,7 +1725,10 @@ void searchTest0() {
}

EchoResponseInterface req = (EchoResponseInterface) assertDoesNotThrow(() -> {
return client.search(indexName0, SearchParams.of(searchParams0));
return client.search(
indexName0,
SearchParams.ofSearchParamsObject(searchParams0)
);
}
);

Expand Down