-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[typescript] fix: enum can not receive stringified class name
#19481
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 |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| generatorName: typescript | ||
| outputDir: samples/openapi3/client/petstore/typescript/builds/enum | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/enum_mapping_test.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/typescript | ||
| additionalProperties: | ||
| platform: deno | ||
| npmName: ts-petstore-client | ||
| projectName: ts-petstore-client | ||
| moduleName: petstore |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| openapi: 3.0.0 | ||
|
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 is an extract of the OpenAPI spec of https://developer.affinity.co/
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. Apologies for the noise that introducing a new sample fixture produces, I couldn't find a sample OpenAPI spec that shows the issue I am trying to fix. If you know of one that has a discriminator which is an enum, please feel free to point me to it, then I might be able to use it instead and reduce the footprint of this PR a bit. |
||
| info: | ||
| title: Sample API to test enum mapping | ||
| description: API description in Markdown. | ||
| version: 1.0.0 | ||
| paths: | ||
| /interactions: | ||
| get: | ||
| summary: Returns all interactions. | ||
| description: Optional extended description in Markdown. | ||
| parameters: | ||
| - in: query | ||
| name: parameter_type_mapping | ||
| schema: | ||
| type: string | ||
| format: special | ||
| responses: | ||
| 200: | ||
| description: OK | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/Interaction' | ||
| components: | ||
| schemas: | ||
| ChatMessage: | ||
| type: "object" | ||
| properties: | ||
| type: | ||
| description: "The type of interaction" | ||
| enum: | ||
| - "chat-message" | ||
| example: "chat-message" | ||
| type: "string" | ||
| message: | ||
| description: "The message content" | ||
| type: "string" | ||
| example: "Hello, how are you?" | ||
| Email: | ||
| type: "object" | ||
| properties: | ||
| type: | ||
| description: "The type of interaction" | ||
| enum: | ||
| - "email" | ||
| example: "email" | ||
| type: "string" | ||
| subject: | ||
| description: "The email subject" | ||
| type: "string" | ||
| example: "Meeting" | ||
| body: | ||
| description: "The email body" | ||
| type: "string" | ||
| example: "Hello, I would like to schedule a meeting with you." | ||
| Interaction: | ||
| required: | ||
| - type | ||
| type: "object" | ||
| discriminator: | ||
| propertyName: "type" | ||
| mapping: | ||
| chat-message: "#/components/schemas/ChatMessage" | ||
| email: "#/components/schemas/Email" | ||
| properties: | ||
| type: | ||
| description: "The type of interaction" | ||
| enum: | ||
| - "call" | ||
| - "email" | ||
| - "meeting" | ||
| - "chat-message" | ||
| example: "meeting" | ||
| type: "string" | ||
| oneOf: | ||
| - $ref: "#/components/schemas/ChatMessage" | ||
| - $ref: "#/components/schemas/Email" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| dist |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # OpenAPI Generator Ignore | ||
| # Generated by openapi-generator https://github.com/openapitools/openapi-generator | ||
|
|
||
| # Use this file to prevent files from being overwritten by the generator. | ||
| # The patterns follow closely to .gitignore or .dockerignore. | ||
|
|
||
| # As an example, the C# client generator defines ApiClient.cs. | ||
| # You can make changes and tell OpenAPI Generator to ignore just this file by uncommenting the following line: | ||
| #ApiClient.cs | ||
|
|
||
| # You can match any string of characters against a directory, file or extension with a single asterisk (*): | ||
| #foo/*/qux | ||
| # The above matches foo/bar/qux and foo/baz/qux, but not foo/bar/baz/qux | ||
|
|
||
| # You can recursively match patterns against a directory, file or extension with a double asterisk (**): | ||
| #foo/**/qux | ||
| # This matches foo/bar/qux, foo/baz/qux, and foo/bar/baz/qux | ||
|
|
||
| # You can also negate patterns with an exclamation (!). | ||
| # For example, you can ignore all files in a docs folder with the file extension .md: | ||
| #docs/*.md | ||
| # Then explicitly reverse the ignore rule for a single file: | ||
| #!docs/README.md |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| .gitignore | ||
| DefaultApi.md | ||
| apis/DefaultApi.ts | ||
| apis/baseapi.ts | ||
| apis/exception.ts | ||
| auth/auth.ts | ||
| configuration.ts | ||
| git_push.sh | ||
| http/http.ts | ||
| http/isomorphic-fetch.ts | ||
| index.ts | ||
| middleware.ts | ||
| models/ChatMessage.ts | ||
| models/Email.ts | ||
| models/Interaction.ts | ||
| models/ObjectSerializer.ts | ||
| models/all.ts | ||
| rxjsStub.ts | ||
| servers.ts | ||
| types/ObjectParamAPI.ts | ||
| types/ObservableAPI.ts | ||
| types/PromiseAPI.ts | ||
| util.ts |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 7.9.0-SNAPSHOT |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # petstore.DefaultApi | ||
|
|
||
| All URIs are relative to *http://localhost* | ||
|
|
||
| Method | HTTP request | Description | ||
| ------------- | ------------- | ------------- | ||
| [**interactionsGet**](DefaultApi.md#interactionsGet) | **GET** /interactions | Returns all interactions. | ||
|
|
||
|
|
||
| # **interactionsGet** | ||
| > Interaction interactionsGet() | ||
|
|
||
| Optional extended description in Markdown. | ||
|
|
||
| ### Example | ||
|
|
||
|
|
||
| ```typescript | ||
| import { petstore } from 'ts-petstore-client'; | ||
| import * as fs from 'fs'; | ||
|
|
||
| const configuration = petstore.createConfiguration(); | ||
| const apiInstance = new petstore.DefaultApi(configuration); | ||
|
|
||
| let body:petstore.DefaultApiInteractionsGetRequest = { | ||
| // string (optional) | ||
| parameterTypeMapping: "parameter_type_mapping_example", | ||
| }; | ||
|
|
||
| apiInstance.interactionsGet(body).then((data:any) => { | ||
| console.log('API called successfully. Returned data: ' + data); | ||
| }).catch((error:any) => console.error(error)); | ||
| ``` | ||
|
|
||
|
|
||
| ### Parameters | ||
|
|
||
| Name | Type | Description | Notes | ||
| ------------- | ------------- | ------------- | ------------- | ||
| **parameterTypeMapping** | [**string**] | | (optional) defaults to undefined | ||
|
|
||
|
|
||
| ### Return type | ||
|
|
||
| **Interaction** | ||
|
|
||
| ### Authorization | ||
|
|
||
| No authorization required | ||
|
|
||
| ### HTTP request headers | ||
|
|
||
| - **Content-Type**: Not defined | ||
| - **Accept**: application/json | ||
|
|
||
|
|
||
| ### HTTP response details | ||
| | Status code | Description | Response headers | | ||
| |-------------|-------------|------------------| | ||
| **200** | OK | - | | ||
|
|
||
| [[Back to top]](#) [[Back to API list]](README.md#documentation-for-api-endpoints) [[Back to Model list]](README.md#documentation-for-models) [[Back to README]](README.md) | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // TODO: better import syntax? | ||
| import {BaseAPIRequestFactory, RequiredError, COLLECTION_FORMATS} from './baseapi.ts'; | ||
| import {Configuration} from '../configuration.ts'; | ||
| import {RequestContext, HttpMethod, ResponseContext, HttpFile, HttpInfo} from '../http/http.ts'; | ||
| import {ObjectSerializer} from '../models/ObjectSerializer.ts'; | ||
| import {ApiException} from './exception.ts'; | ||
| import {canConsumeForm, isCodeInRange} from '../util.ts'; | ||
| import {SecurityAuthentication} from '../auth/auth.ts'; | ||
|
|
||
|
|
||
| import { Interaction } from '../models/Interaction.ts'; | ||
|
|
||
| /** | ||
| * no description | ||
| */ | ||
| export class DefaultApiRequestFactory extends BaseAPIRequestFactory { | ||
|
|
||
| /** | ||
| * Optional extended description in Markdown. | ||
| * Returns all interactions. | ||
| * @param parameterTypeMapping | ||
| */ | ||
| public async interactionsGet(parameterTypeMapping?: string, _options?: Configuration): Promise<RequestContext> { | ||
| let _config = _options || this.configuration; | ||
|
|
||
|
|
||
| // Path Params | ||
| const localVarPath = '/interactions'; | ||
|
|
||
| // Make Request Context | ||
| const requestContext = _config.baseServer.makeRequestContext(localVarPath, HttpMethod.GET); | ||
| requestContext.setHeaderParam("Accept", "application/json, */*;q=0.8") | ||
|
|
||
| // Query Params | ||
| if (parameterTypeMapping !== undefined) { | ||
| requestContext.setQueryParam("parameter_type_mapping", ObjectSerializer.serialize(parameterTypeMapping, "string", "special")); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| const defaultAuth: SecurityAuthentication | undefined = _options?.authMethods?.default || this.configuration?.authMethods?.default | ||
| if (defaultAuth?.applySecurityAuthentication) { | ||
| await defaultAuth?.applySecurityAuthentication(requestContext); | ||
| } | ||
|
|
||
| return requestContext; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| export class DefaultApiResponseProcessor { | ||
|
|
||
| /** | ||
| * Unwraps the actual response sent by the server from the response context and deserializes the response content | ||
| * to the expected objects | ||
| * | ||
| * @params response Response returned by the server for a request to interactionsGet | ||
| * @throws ApiException if the response code was not in [200, 299] | ||
| */ | ||
| public async interactionsGetWithHttpInfo(response: ResponseContext): Promise<HttpInfo<Interaction >> { | ||
| const contentType = ObjectSerializer.normalizeMediaType(response.headers["content-type"]); | ||
| if (isCodeInRange("200", response.httpStatusCode)) { | ||
| const body: Interaction = ObjectSerializer.deserialize( | ||
| ObjectSerializer.parse(await response.body.text(), contentType), | ||
| "Interaction", "" | ||
| ) as Interaction; | ||
| return new HttpInfo(response.httpStatusCode, response.headers, response.body, body); | ||
| } | ||
|
|
||
| // Work around for missing responses in specification, e.g. for petstore.yaml | ||
| if (response.httpStatusCode >= 200 && response.httpStatusCode <= 299) { | ||
| const body: Interaction = ObjectSerializer.deserialize( | ||
| ObjectSerializer.parse(await response.body.text(), contentType), | ||
| "Interaction", "" | ||
| ) as Interaction; | ||
| return new HttpInfo(response.httpStatusCode, response.headers, response.body, body); | ||
| } | ||
|
|
||
| throw new ApiException<string | Blob | undefined>(response.httpStatusCode, "Unknown API Status Code!", await response.getBodyAsAny(), response.headers); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { Configuration } from '../configuration.ts' | ||
|
|
||
| /** | ||
| * | ||
| * @export | ||
| */ | ||
| export const COLLECTION_FORMATS = { | ||
| csv: ",", | ||
| ssv: " ", | ||
| tsv: "\t", | ||
| pipes: "|", | ||
| }; | ||
|
|
||
|
|
||
| /** | ||
| * | ||
| * @export | ||
| * @class BaseAPI | ||
| */ | ||
| export class BaseAPIRequestFactory { | ||
|
|
||
| constructor(protected configuration: Configuration) { | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * | ||
| * @export | ||
| * @class RequiredError | ||
| * @extends {Error} | ||
| */ | ||
| export class RequiredError extends Error { | ||
| name: "RequiredError" = "RequiredError"; | ||
| constructor(public api: string, public method: string, public field: string) { | ||
| super("Required parameter " + field + " was null or undefined when calling " + api + "." + method + "."); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /** | ||
| * Represents an error caused by an api call i.e. it has attributes for a HTTP status code | ||
| * and the returned body object. | ||
| * | ||
| * Example | ||
| * API returns a ErrorMessageObject whenever HTTP status code is not in [200, 299] | ||
| * => ApiException(404, someErrorMessageObject) | ||
| * | ||
| */ | ||
| export class ApiException<T> extends Error { | ||
| public constructor(public code: number, message: string, public body: T, public headers: { [key: string]: string; }) { | ||
| super("HTTP-Code: " + code + "\nMessage: " + message + "\nBody: " + JSON.stringify(body) + "\nHeaders: " + | ||
| JSON.stringify(headers)) | ||
| } | ||
| } |
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 the meaningful change of this PR.
I can see here:
openapi-generator/modules/openapi-generator/src/main/resources/Java/pojo.mustache
Lines 95 to 99 in b4357ed
that other generators guard the opposite case. If I misunderstand what the fix is, please let me know. An alternative could also be to make the discriminator type always a string/symbol and only assign the stringified value of each enum value.
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.
so what would be the
discriminator.isEnumcase?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.
The discriminator is the
typemember andtypeis of typeInteractionTypeEnum, thus the template stringdiscriminator.isEnumwould evaluate totrue, hence with the template negation, it would not emit the guarded class name assignment (e.g. it would omitthis.{{discriminatorName}} = "{{classname}}";)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.
basically before the change we'd produce code that looks like this:
(note how the string
"Interaction"is not a valid value ofInteractionTypeEnum)After the changes in this PR it would be:
E.g. the meaningful diff is:
export class Interaction { 'type': InteractionTypeEnum; static readonly discriminator: string | undefined = "type"; public constructor() { - this.type = "Interaction"; } }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.
thanks for the explanation! I wonder why there even is such a default value assigned, do you happen to know?
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.
I do not, sorry, I am assuming it is a remnant from making sure each class has a sane default value to discriminate on, however the implementation of it doesn't make sense. It works fine when the discriminator is of string type, which is the majority of implementations I've seen, but it obviously doesn't work any more as soon as it's a fixed enum.