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

feat(oas): support application/x-www-form-urlencoded OAS style serialization #227

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ostridm
Copy link
Contributor

@ostridm ostridm commented Feb 26, 2024

closes #226

@ostridm ostridm added the Type: enhancement New feature or request. label Feb 26, 2024
@ostridm ostridm self-assigned this Feb 26, 2024
Copy link

codeclimate bot commented Feb 26, 2024

Code Climate has analyzed commit 3d5d570 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 96.2% (50% is the threshold).

This pull request will bring the total coverage in the repository to 92.2% (0.1% change).

View more on Code Climate.

@ostridm ostridm requested a review from derevnjuk February 26, 2024 12:45
Comment on lines +41 to +43
value: unknown,
fields?: Record<string, OpenAPIV3.EncodingObject>,
schema?: OpenAPIV3.SchemaObject | OpenAPIV2.SchemaObject
Copy link
Member

Choose a reason for hiding this comment

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

Convert args to an object and introduce interface

return this.findSerializer(style)?.serialize(
{
key,
value: stringifyObject ? JSON.stringify(val) : val
Copy link
Member

Choose a reason for hiding this comment

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

When dealing with complex values that are not arrays of primitive types or simple objects with primitive fields, you have to rely on the contentType. It is not necessary a JSON

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, when contentType is specified it has already handled in Oas3RequestBodyConverter.encodeProperties and thus here we receive string primitive.

Copy link
Member

Choose a reason for hiding this comment

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

The contentMediaType can be present in the schema itself, which is intended to be handled if there are no encodings or media type object keys. Even beyond this, the behavior for nested objects and arrays is undefined in the specification; it is better to simply ignore values in such cases to avoid API misbehaviour.

): value is null | undefined | string | boolean | number | bigint | symbol =>
// ADHOC: typeof null === 'object'
value === null ||
value === undefined ||
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
value === undefined ||
typeof value === 'undefined' ||

]
})
encoding.contentType
? this.encodeValue({
Copy link
Member

Choose a reason for hiding this comment

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

The encodeValue should manage this scenario. Ensure it is able to accept undefined as a valid contentType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a possibility to encode some primitive or array to string. See (#227 (comment)) which will prevent correct serialization due to loss of type info

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. However, you should figure out how to handle the contentMediaType in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: enhancement New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add application/x-www-form-urlencoded OAS style serialization support
2 participants