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.

Sign up for GitHub

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

Jump to bottom

Fix collectionFormat issue in header #1878

Merged
merged 7 commits into from
Jun 6, 2023
Merged

Fix collectionFormat issue in header #1878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b95fc23
Merge branch 'main' into fix-csv-default-issue
MaryGao Jun 5, 2023
9f8428a
Add helper method for header csv format
MaryGao Jun 5, 2023
e808d05
Update the test cases in swagger
MaryGao Jun 5, 2023
452eb71
Update the swagger side change
MaryGao Jun 5, 2023
66354f8
Revert changes
MaryGao Jun 5, 2023
b23b3d5
Revert changes
MaryGao Jun 5, 2023
5247311
Update the generated code
MaryGao Jun 5, 2023
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
2 changes: 1 addition & 1 deletion packages/autorest.typescript/src/restLevelClient/transforms/transformParameterTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,4 @@ function getAllOperations(model: CodeModel): Operation[] {
}

return operations;
}
}
Copy link
Contributor Author

@MaryGao MaryGao Jun 6, 2023 •
edited
Loading

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.

7 changes: 2 additions & 5 deletions ...est.typescript/test/rlcIntegration/generated/multipleUrlParameters/src/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function buildMultiCollection(
queryParameters: string[],
parameterName: string
) {
return queryParameters
export function buildMultiCollection(items: string[], parameterName: string) {
return items
.map((item, index) => {
if (index === 0) {
return item;
Expand Down
12 changes: 6 additions & 6 deletions packages/autorest.typescript/test/rlcIntegration/generated/urlRest/src/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function buildPipeCollection(queryParameters: string[]): string {
return queryParameters.join("|");
export function buildPipeCollection(items: string[]): string {
return items.join("|");
}

export function buildSsvCollection(queryParameters: string[]): string {
return queryParameters.join(" ");
export function buildSsvCollection(items: string[]): string {
return items.join(" ");
}

export function buildTsvCollection(queryParameters: string[]) {
return queryParameters.join("\t");
export function buildTsvCollection(items: string[]) {
return items.join("\t");
}
2 changes: 1 addition & 1 deletion ...ript/test/smoke/generated/agrifood-data-plane/review/agrifood-data-plane.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ export interface BoundaryOverlapResponseOutput {
export type BoundaryResourceMergeAndPatch = Partial<Boundary>;

// @public (undocumented)
export function buildMultiCollection(queryParameters: string[], parameterName: string): string;
export function buildMultiCollection(items: string[], parameterName: string): string;

// @public
export interface CascadeDeleteJobOutput {
Expand Down
7 changes: 2 additions & 5 deletions packages/autorest.typescript/test/smoke/generated/agrifood-data-plane/src/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function buildMultiCollection(
queryParameters: string[],
parameterName: string
) {
return queryParameters
export function buildMultiCollection(items: string[], parameterName: string) {
return items
.map((item, index) => {
if (index === 0) {
return item;
Expand Down
4 changes: 3 additions & 1 deletion packages/rlc-common/src/buildIndexFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { Project, SourceFile } from "ts-morph";
import { NameType, normalizeName } from "./helpers/nameUtils.js";
import {
hasCsvCollection,
hasInputModels,
hasMultiCollection,
hasOutputModels,
Expand Down Expand Up @@ -134,7 +135,8 @@ function generateRLCIndexForMultiClient(file: SourceFile, model: RLCModel) {
hasMultiCollection(model) ||
hasSsvCollection(model) ||
hasPipeCollection(model) ||
hasTsvCollection(model)
hasTsvCollection(model) ||
hasCsvCollection(model)
) {
file.addImportDeclaration({
namespaceImport: "SerializeHelper",
Expand Down
5 changes: 5 additions & 0 deletions packages/rlc-common/src/buildSerializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import * as path from "path";
// @ts-ignore: to fix the handlebars issue
import hbs from "handlebars";
import {
hasCsvCollection,
hasMultiCollection,
hasPipeCollection,
hasSsvCollection,
hasTsvCollection
} from "./helpers/operationHelpers.js";
import {
buildCsvCollectionContent,
buildMultiCollectionContent,
buildPipeCollectionContent,
buildSsvCollectionContent,
Expand All @@ -29,6 +31,9 @@ export function buildSerializeHelper(model: RLCModel) {
if (hasTsvCollection(model)) {
serializeHelperContent += "\n" + buildTsvCollectionContent;
}
if (hasCsvCollection(model)) {
serializeHelperContent += "\n" + buildCsvCollectionContent;
}
if (serializeHelperContent !== "") {
const readmeFileContents = hbs.compile(serializeHelperContent, {
noEscape: true
Expand Down
4 changes: 4 additions & 0 deletions packages/rlc-common/src/helpers/operationHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ export function hasTsvCollection(model: RLCModel) {
return Boolean(model.helperDetails?.hasTsvCollection);
}

export function hasCsvCollection(model: RLCModel) {
return Boolean(model.helperDetails?.hasCsvCollection);
}

export function hasUnexpectedHelper(model: RLCModel) {
const pathDictionary = model.paths;
for (const details of Object.values(pathDictionary)) {
Expand Down
1 change: 1 addition & 0 deletions packages/rlc-common/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface HelperFunctionDetails {
hasPipeCollection?: boolean;
hasSsvCollection?: boolean;
hasTsvCollection?: boolean;
hasCsvCollection?: boolean;
}

export interface PagingDetails {
Expand Down
21 changes: 13 additions & 8 deletions packages/rlc-common/src/static/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.

items: string[],
parameterName: string
) {
return queryParameters
return items
.map((item, index) => {
if (index === 0) {
return item;
Expand All @@ -14,16 +14,21 @@ export function buildMultiCollection(
}`;

export const buildPipeCollectionContent = `
export function buildPipeCollection(queryParameters: string[]): string {
return queryParameters.join("|");
export function buildPipeCollection(items: string[]): string {
return items.join("|");
}`;

export const buildSsvCollectionContent = `
export function buildSsvCollection(queryParameters: string[]): string {
return queryParameters.join(" ");
export function buildSsvCollection(items: string[]): string {
return items.join(" ");
}`;

export const buildTsvCollectionContent = `
export function buildTsvCollection(queryParameters: string[]) {
return queryParameters.join("\\t");
export function buildTsvCollection(items: string[]) {
return items.join("\\t");
}`;

export const buildCsvCollectionContent = `
export function buildCsvCollection(items: string[]) {
return items.join(",");
}`;
2 changes: 1 addition & 1 deletion ...est/test/collectionFormat/generated/typespec-ts/review/collection-format.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { RequestParameters } from '@azure-rest/core-client';
import { StreamableMethod } from '@azure-rest/core-client';

// @public (undocumented)
export function buildMultiCollection(queryParameters: string[], parameterName: string): string;
export function buildMultiCollection(items: string[], parameterName: string): string;

// @public (undocumented)
export type CollectionFormatTestServiceClient = Client & {
Expand Down
7 changes: 2 additions & 5 deletions packages/typespec-test/test/collectionFormat/generated/typespec-ts/src/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function buildMultiCollection(
queryParameters: string[],
parameterName: string
) {
return queryParameters
export function buildMultiCollection(items: string[], parameterName: string) {
return items
.map((item, index) => {
if (index === 0) {
return item;
Expand Down
2 changes: 1 addition & 1 deletion ...est/translator/generated/typespec-ts/review/cognitiveservices-translator.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export interface BreakSentenceQueryParamProperties {
}

// @public (undocumented)
export function buildMultiCollection(queryParameters: string[], parameterName: string): string;
export function buildMultiCollection(items: string[], parameterName: string): string;

// @public
export interface CommonScriptModelOutput {
Expand Down
7 changes: 2 additions & 5 deletions packages/typespec-test/test/translator/generated/typespec-ts/src/serializeHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export function buildMultiCollection(
queryParameters: string[],
parameterName: string
) {
return queryParameters
export function buildMultiCollection(items: string[], parameterName: string) {
return items
.map((item, index) => {
if (index === 0) {
return item;
Expand Down
10 changes: 9 additions & 1 deletion packages/typespec-ts/src/transform/transformHelperFunctionDetails.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ function extractSpecialSerializeInfo(
let hasPipeCollection = false;
let hasTsvCollection = false;
let hasSsvCollection = false;
let hasCsvCollection = false;
const operationGroups = listOperationGroups(dpgContext, client);
for (const operationGroup of operationGroups) {
const operations = listOperationsInOperationGroup(
Expand All @@ -216,6 +217,9 @@ function extractSpecialSerializeInfo(
hasSsvCollection = hasSsvCollection
? hasSsvCollection
: serializeInfo.hasSsvCollection;
hasCsvCollection = hasCsvCollection
? hasCsvCollection
: serializeInfo.hasCsvCollection;
});
}
}
Expand All @@ -236,12 +240,16 @@ function extractSpecialSerializeInfo(
hasSsvCollection = hasSsvCollection
? hasSsvCollection
: serializeInfo.hasSsvCollection;
hasCsvCollection = hasCsvCollection
? hasCsvCollection
: serializeInfo.hasCsvCollection;
});
}
return {
hasMultiCollection,
hasPipeCollection,
hasTsvCollection,
hasSsvCollection
hasSsvCollection,
hasCsvCollection
};
}
11 changes: 10 additions & 1 deletion packages/typespec-ts/src/transform/transformParameters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ function getParameterMetadata(
serializeInfo.hasMultiCollection ||
serializeInfo.hasPipeCollection ||
serializeInfo.hasSsvCollection ||
serializeInfo.hasTsvCollection
serializeInfo.hasTsvCollection ||
serializeInfo.hasCsvCollection
) {
type = "string";
description += ` This parameter needs to be formatted as ${serializeInfo.collectionInfo.join(
Expand Down Expand Up @@ -495,6 +496,7 @@ export function getSpecialSerializeInfo(parameter: HttpOperationParameter) {
let hasPipeCollection = false;
let hasSsvCollection = false;
let hasTsvCollection = false;
let hasCsvCollection = false;
const descriptions = [];
const collectionInfo = [];
if (
Expand Down Expand Up @@ -522,11 +524,18 @@ export function getSpecialSerializeInfo(parameter: HttpOperationParameter) {
descriptions.push("buildPipeCollection");
collectionInfo.push("pipe");
}

if (parameter.type === "header" && (parameter as any).format === "csv") {
hasCsvCollection = true;
descriptions.push("buildCsvCollection");
collectionInfo.push("csv");
}
return {
hasMultiCollection,
hasPipeCollection,
hasSsvCollection,
hasTsvCollection,
hasCsvCollection,
descriptions,
collectionInfo
};
Expand Down
Loading
Loading