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 collectionFormat issue in header #1878

Merged
merged 7 commits into from
Jun 6, 2023

Conversation

MaryGao
Copy link
Contributor

@MaryGao MaryGao commented Jun 5, 2023

When touching the collectionFormat integration cases we generate string[] for header with format defined, but I notice that RawHttpHeadersInput only allows for string|number|boolean which means there would exist type inference conflict for this property.

https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/core/core-rest-pipeline/src/interfaces.ts#L12-L15

To fix that we need to build a helper function for the default csv format for header.

@MaryGao MaryGao changed the title Fix csv default issue Fix collectionFormat issue in header Jun 5, 2023
@MaryGao MaryGao marked this pull request as ready for review June 5, 2023 15:11
@@ -304,4 +304,4 @@ function getAllOperations(model: CodeModel): Operation[] {
}

return operations;
}
}
Copy link
Contributor Author

@MaryGao MaryGao Jun 6, 2023

Choose a reason for hiding this comment

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

I didn't support this case in swagger side, because I didn't find a valid case anywhere and when I tried to build one the autorest would complain Unknown primitive schema array.

I guess only primitive types are supported in header for autorest.

@@ -1,9 +1,9 @@
export const buildMultiCollectionContent = `
export function buildMultiCollection(
queryParameters: string[],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change the param name to item because it is not limited to query params.

@MaryGao MaryGao merged commit 46674e7 into Azure:main Jun 6, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants