-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[fix][elixir] wrong typespec generation for all-of with single ref #21139
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,44 +180,55 @@ public ElixirClientCodegen() { | |
| */ | ||
| languageSpecificPrimitives = new HashSet<>( | ||
| Arrays.asList( | ||
| "Integer", | ||
| "Float", | ||
| "Decimal", | ||
| "Boolean", | ||
| "String", | ||
| "List", | ||
| "Atom", | ||
| "Map", | ||
| "AnyType", | ||
| "Tuple", | ||
| "PID", | ||
| // This is a workaround, since the DefaultCodeGen uses our elixir TypeSpec | ||
| // datetype to evaluate the primitive | ||
| "integer()", | ||
| "float()", | ||
| "number()", | ||
| "boolean()", | ||
| "String.t", | ||
| "Date.t", | ||
| "DateTime.t", | ||
| "binary()", | ||
| "list()", | ||
| "map()", | ||
| "any()")); | ||
| "any()", | ||
| "nil")); | ||
|
|
||
| // ref: | ||
| // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types | ||
| typeMapping = new HashMap<>(); | ||
| typeMapping.put("integer", "Integer"); | ||
| typeMapping.put("long", "Integer"); | ||
| typeMapping.put("number", "Float"); | ||
| typeMapping.put("float", "Float"); | ||
| typeMapping.put("double", "Float"); | ||
| typeMapping.put("string", "String"); | ||
| typeMapping.put("byte", "Integer"); | ||
| typeMapping.put("boolean", "Boolean"); | ||
| typeMapping.put("Date", "Date"); | ||
| typeMapping.put("DateTime", "DateTime"); | ||
| typeMapping.put("file", "String"); | ||
| typeMapping.put("map", "Map"); | ||
| typeMapping.put("array", "List"); | ||
| typeMapping.put("list", "List"); | ||
| typeMapping.put("object", "Map"); | ||
| typeMapping.put("binary", "String"); | ||
| typeMapping.put("ByteArray", "String"); | ||
| typeMapping.put("UUID", "String"); | ||
| typeMapping.put("URI", "String"); | ||
| // primitive types | ||
| typeMapping.put("string", "String.t"); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now, we are truly mapping to Elixir built-in types. |
||
| typeMapping.put("number", "number()"); | ||
| typeMapping.put("integer", "integer()"); | ||
| typeMapping.put("boolean", "boolean()"); | ||
| typeMapping.put("array", "list()"); | ||
| typeMapping.put("object", "map()"); | ||
| typeMapping.put("map", "map()"); | ||
| typeMapping.put("null", "nil"); | ||
| // string formats | ||
| typeMapping.put("byte", "String.t"); | ||
| typeMapping.put("binary", "binary()"); | ||
| typeMapping.put("password", "String.t"); | ||
| typeMapping.put("uuid", "String.t"); | ||
| typeMapping.put("email", "String.t"); | ||
| typeMapping.put("uri", "String.t"); | ||
| typeMapping.put("file", "String.t"); | ||
| // integer formats | ||
| typeMapping.put("int32", "integer()"); | ||
| typeMapping.put("int64", "integer()"); | ||
| typeMapping.put("long", "integer()"); | ||
| // float formats | ||
| typeMapping.put("float", "float()"); | ||
| typeMapping.put("double", "float()"); | ||
| typeMapping.put("decimal", "float()"); | ||
| // date-time formats | ||
| typeMapping.put("date", "Date.t"); | ||
| typeMapping.put("date-time", "DateTime.t"); | ||
| // other | ||
| typeMapping.put("ByteArray", "binary()"); | ||
| typeMapping.put("DateTime", "DateTime.t"); | ||
| typeMapping.put("UUID", "String.t"); | ||
|
|
||
|
|
||
| cliOptions.add(new CliOption(CodegenConstants.INVOKER_PACKAGE, | ||
| "The main namespace to use for all classes. e.g. Yay.Pets")); | ||
|
|
@@ -570,49 +581,19 @@ public String toOperationId(String operationId) { | |
| */ | ||
| @Override | ||
| public String getTypeDeclaration(Schema p) { | ||
| if (ModelUtils.isAnyType(p)) { | ||
| return "any()"; | ||
| } else if(ModelUtils.isFreeFormObject(p, null)) { | ||
| return "%{optional(String.t) => any()}"; | ||
| } else if (ModelUtils.isArraySchema(p)) { | ||
| if (ModelUtils.isArraySchema(p)) { | ||
| Schema inner = ModelUtils.getSchemaItems(p); | ||
| return "[" + getTypeDeclaration(inner) + "]"; | ||
| } else if (ModelUtils.isMapSchema(p)) { | ||
| Schema inner = ModelUtils.getAdditionalProperties(p); | ||
| return "%{optional(String.t) => " + getTypeDeclaration(inner) + "}"; | ||
| } else if (ModelUtils.isPasswordSchema(p)) { | ||
| return "String.t"; | ||
| } else if (ModelUtils.isEmailSchema(p)) { | ||
| return "String.t"; | ||
| } else if (ModelUtils.isByteArraySchema(p)) { | ||
| return "binary()"; | ||
| } else if (ModelUtils.isUUIDSchema(p)) { | ||
| return "String.t"; | ||
| } else if (ModelUtils.isDateSchema(p)) { | ||
| return "Date.t"; | ||
| } else if (ModelUtils.isDateTimeSchema(p)) { | ||
| return "DateTime.t"; | ||
| } else if (ModelUtils.isObjectSchema(p)) { | ||
| return "map()"; | ||
| } else if (ModelUtils.isIntegerSchema(p)) { | ||
| return "integer()"; | ||
| } else if (ModelUtils.isNumberSchema(p)) { | ||
| return "float()"; | ||
| } else if (ModelUtils.isBinarySchema(p) || ModelUtils.isFileSchema(p)) { | ||
| return "String.t"; | ||
| } else if (ModelUtils.isBooleanSchema(p)) { | ||
| return "boolean()"; | ||
| } else if (!StringUtils.isEmpty(p.get$ref())) { | ||
| switch (super.getTypeDeclaration(p)) { | ||
| case "String": | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With better type mappings, code like this one (which was incomplete and should have also included other types such as |
||
| return "String.t"; | ||
| default: | ||
| return this.moduleName + ".Model." + super.getTypeDeclaration(p) + ".t"; | ||
| String refType = super.getTypeDeclaration(p); | ||
| if (languageSpecificPrimitives.contains(refType)) { | ||
| return refType; | ||
| } else { | ||
| return this.moduleName + ".Model." + refType + ".t"; | ||
| } | ||
| } else if (ModelUtils.isFileSchema(p)) { | ||
| return "String.t"; | ||
| } else if (ModelUtils.isStringSchema(p)) { | ||
| return "String.t"; | ||
| } else if (p.getType() == null) { | ||
| return "any()"; | ||
| } | ||
|
|
@@ -630,14 +611,11 @@ public String getTypeDeclaration(Schema p) { | |
| @Override | ||
| public String getSchemaType(Schema p) { | ||
| String openAPIType = super.getSchemaType(p); | ||
| String type = null; | ||
| if (typeMapping.containsKey(openAPIType)) { | ||
| type = typeMapping.get(openAPIType); | ||
| if (languageSpecificPrimitives.contains(type)) | ||
| return toModelName(type); | ||
| } else | ||
| type = openAPIType; | ||
| return toModelName(type); | ||
| return typeMapping.get(openAPIType); | ||
| } else { | ||
| return toModelName(openAPIType); | ||
| } | ||
| } | ||
|
|
||
| class ExtendedCodegenResponse extends CodegenResponse { | ||
|
|
@@ -784,24 +762,6 @@ public ExtendedCodegenOperation(CodegenOperation o) { | |
| this.operationIdCamelCase = o.operationIdCamelCase; | ||
| } | ||
|
|
||
| private void translateBaseType(StringBuilder returnEntry, String baseType) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method wasn't used anywhere in the generator. |
||
| switch (baseType) { | ||
| case "AnyType": | ||
| returnEntry.append("any()"); | ||
| break; | ||
| case "Boolean": | ||
| returnEntry.append("boolean()"); | ||
| break; | ||
| case "Float": | ||
| returnEntry.append("float()"); | ||
| break; | ||
| default: | ||
| returnEntry.append(baseType); | ||
| returnEntry.append(".t"); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| public String typespec() { | ||
| StringBuilder sb = new StringBuilder("@spec "); | ||
| sb.append(underscore(operationId)); | ||
|
|
@@ -819,16 +779,10 @@ public String typespec() { | |
| for (CodegenResponse response : this.responses) { | ||
| ExtendedCodegenResponse exResponse = (ExtendedCodegenResponse) response; | ||
| StringBuilder returnEntry = new StringBuilder(); | ||
| if (exResponse.baseType == null) { | ||
| returnEntry.append("nil"); | ||
| } else if (exResponse.containerType == null) { // not container (array, map, set) | ||
| returnEntry.append(normalizeTypeName(exResponse.dataType, exResponse.primitiveType)); | ||
| if (exResponse.schema != null) { | ||
| returnEntry.append(getTypeDeclaration((Schema) exResponse.schema)); | ||
| } else { | ||
| if (exResponse.containerType.equals("array") || exResponse.containerType.equals("set")) { | ||
| returnEntry.append(exResponse.dataType); | ||
| } else if (exResponse.containerType.equals("map")) { | ||
| returnEntry.append("map()"); | ||
| } | ||
| returnEntry.append(normalizeTypeName(exResponse.dataType, exResponse.primitiveType)); | ||
| } | ||
| uniqueResponseTypes.add(returnEntry.toString()); | ||
| } | ||
|
|
@@ -845,14 +799,11 @@ private String normalizeTypeName(String baseType, boolean isPrimitive) { | |
| if (baseType == null) { | ||
| return "nil"; | ||
| } | ||
| if (isPrimitive || "String.t".equals(baseType)) { | ||
| if (isPrimitive || languageSpecificPrimitives.contains(baseType)) { | ||
| return baseType; | ||
| } | ||
| if (!baseType.startsWith(moduleName + ".Model.")) { | ||
| baseType = moduleName + ".Model." + baseType; | ||
| } | ||
| if (!baseType.endsWith(".t")) { | ||
| baseType += ".t"; | ||
| baseType = moduleName + ".Model." + baseType + ".t"; | ||
| } | ||
| return baseType; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,60 @@ defmodule OpenapiPetstore.Api.Fake do | |
| alias OpenapiPetstore.Connection | ||
| import OpenapiPetstore.RequestBuilder | ||
|
|
||
| @doc """ | ||
|
|
||
| ### Parameters | ||
|
|
||
| - `connection` (OpenapiPetstore.Connection): Connection to server | ||
| - `opts` (keyword): Optional parameters | ||
|
|
||
| ### Returns | ||
|
|
||
| - `{:ok, OpenapiPetstore.Model.Foo.t}` on success | ||
| - `{:error, Tesla.Env.t}` on failure | ||
| """ | ||
| @spec fake_all_of_with_local_single_ref_get(Tesla.Env.client, keyword()) :: {:ok, any()} | {:error, Tesla.Env.t} | ||
| def fake_all_of_with_local_single_ref_get(connection, _opts \\ []) do | ||
| request = | ||
| %{} | ||
| |> method(:get) | ||
| |> url("/fake/all-of-with-local-single-ref") | ||
| |> Enum.into([]) | ||
|
|
||
| connection | ||
| |> Connection.request(request) | ||
| |> evaluate_response([ | ||
| {200, OpenapiPetstore.Model.Foo} | ||
| ]) | ||
| end | ||
|
|
||
| @doc """ | ||
|
|
||
| ### Parameters | ||
|
|
||
| - `connection` (OpenapiPetstore.Connection): Connection to server | ||
| - `opts` (keyword): Optional parameters | ||
|
|
||
| ### Returns | ||
|
|
||
| - `{:ok, OpenapiPetstore.Model.AllOfWithSingleRef.t}` on success | ||
| - `{:error, Tesla.Env.t}` on failure | ||
| """ | ||
| @spec fake_all_of_with_remote_single_ref_get(Tesla.Env.client, keyword()) :: {:ok, OpenapiPetstore.Model.AllOfWithSingleRef.t} | {:error, Tesla.Env.t} | ||
| def fake_all_of_with_remote_single_ref_get(connection, _opts \\ []) do | ||
| request = | ||
| %{} | ||
| |> method(:get) | ||
| |> url("/fake/all-of-with-remote-single-ref") | ||
| |> Enum.into([]) | ||
|
|
||
| connection | ||
| |> Connection.request(request) | ||
| |> evaluate_response([ | ||
| {200, OpenapiPetstore.Model.AllOfWithSingleRef} | ||
| ]) | ||
| end | ||
|
|
||
| @doc """ | ||
| for Java apache and Java native, test toUrlQueryString for maps with BegDecimal keys | ||
|
|
||
|
|
@@ -180,14 +234,14 @@ defmodule OpenapiPetstore.Api.Fake do | |
|
|
||
| - `connection` (OpenapiPetstore.Connection): Connection to server | ||
| - `opts` (keyword): Optional parameters | ||
| - `:body` (float()): Input number as post body | ||
| - `:body` (number()): Input number as post body | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| ### Returns | ||
|
|
||
| - `{:ok, float()}` on success | ||
| - `{:ok, number()}` on success | ||
| - `{:error, Tesla.Env.t}` on failure | ||
| """ | ||
| @spec fake_outer_number_serialize(Tesla.Env.client, keyword()) :: {:ok, float()} | {:error, Tesla.Env.t} | ||
| @spec fake_outer_number_serialize(Tesla.Env.client, keyword()) :: {:ok, number()} | {:error, Tesla.Env.t} | ||
| def fake_outer_number_serialize(connection, opts \\ []) do | ||
| optional_params = %{ | ||
| :body => :body | ||
|
|
@@ -464,7 +518,7 @@ defmodule OpenapiPetstore.Api.Fake do | |
| ### Parameters | ||
|
|
||
| - `connection` (OpenapiPetstore.Connection): Connection to server | ||
| - `number` (float()): None | ||
| - `number` (number()): None | ||
| - `double` (float()): None | ||
| - `pattern_without_delimiter` (String.t): None | ||
| - `byte` (binary()): None | ||
|
|
@@ -485,7 +539,7 @@ defmodule OpenapiPetstore.Api.Fake do | |
| - `{:ok, nil}` on success | ||
| - `{:error, Tesla.Env.t}` on failure | ||
| """ | ||
| @spec test_endpoint_parameters(Tesla.Env.client, float(), float(), String.t, binary(), keyword()) :: {:ok, nil} | {:error, Tesla.Env.t} | ||
| @spec test_endpoint_parameters(Tesla.Env.client, number(), float(), String.t, binary(), keyword()) :: {:ok, nil} | {:error, Tesla.Env.t} | ||
| def test_endpoint_parameters(connection, number, double, pattern_without_delimiter, byte, opts \\ []) do | ||
| optional_params = %{ | ||
| :integer => :form, | ||
|
|
@@ -623,7 +677,7 @@ defmodule OpenapiPetstore.Api.Fake do | |
| ### Parameters | ||
|
|
||
| - `connection` (OpenapiPetstore.Connection): Connection to server | ||
| - `body` (%{optional(String.t) => any()}): request body | ||
| - `body` (map()): request body | ||
| - `opts` (keyword): Optional parameters | ||
|
|
||
| ### Returns | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more idiomatic and allows us to better identify what schemas may need a custom model doing something along the lines of
languageSpecificPrimitives.contains(type)