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

add a cache for parameters/responses/operations #6030

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

add a cache for parameters/responses/operations #6030

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
cd73d9d
add a cache for parameters
ArcturusZhang Feb 17, 2025
865a6f6
add cache for response
ArcturusZhang Feb 17, 2025
9d4bcb4
remove all references for isResourceParameter in the csharp part
ArcturusZhang Feb 17, 2025
c7c55fd
Merge branch 'main' into add-cache-for-operations
ArcturusZhang Feb 18, 2025
9aed293
Merge branch 'main' into add-cache-for-operations
ArcturusZhang Feb 21, 2025
3b1b1df
fix compilation issues
ArcturusZhang Feb 21, 2025
f0e1003
Merge branch 'main' into add-cache-for-operations
ArcturusZhang Feb 21, 2025
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
8 changes: 2 additions & 6 deletions packages/http-client-csharp/emitter/src/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { Logger } from "./lib/logger.js";
import { execAsync } from "./lib/utils.js";
import { _resolveOutputFolder, CSharpEmitterOptions, resolveOptions } from "./options.js";
import { defaultSDKContextOptions } from "./sdk-context-options.js";
import { CSharpEmitterContext } from "./sdk-context.js";
import { createSdkTypeCache, CSharpEmitterContext } from "./sdk-context.js";
import { CodeModel } from "./type/code-model.js";
import { Configuration } from "./type/configuration.js";

Expand Down Expand Up @@ -71,11 +71,7 @@ export async function $onEmit(context: EmitContext<CSharpEmitterOptions>) {
defaultSDKContextOptions,
)),
logger: logger,
__typeCache: {
types: new Map(),
models: new Map(),
enums: new Map(),
},
__typeCache: createSdkTypeCache(),
};
program.reportDiagnostics(sdkContext.diagnostics);

Expand Down
1 change: 0 additions & 1 deletion packages/http-client-csharp/emitter/src/lib/client-model-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ export function createModel(sdkContext: CSharpEmitterContext): CodeModel {
Type: parameterType,
Location: RequestLocation.Uri,
IsApiVersion: parameter.isApiVersionParam,
IsResourceParameter: false,
Copy link
Member Author

@ArcturusZhang ArcturusZhang Feb 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I removed this parameter because in everywhere of our emitter, we are assigning false to this property, and in all the places in MGC.
In autorest.csharp, the only chance that this parameter is assigned to true is in the converter that we convert from swagger, when the parameter has a certain x-ms- extension.
Therefore I think we should not maintain it any more.

This property has never been assigned to true in tspCodeModel.json, therefore it is fine that the InputParameter class in autorest.csharp could keep this property, but we no longer need it here.

IsContentType: false,
IsRequired: !parameter.optional,
IsEndpoint: isEndpoint,
Expand Down
29 changes: 7 additions & 22 deletions packages/http-client-csharp/emitter/src/lib/example-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import {
SdkDictionaryExampleValue,
SdkExampleValue,
SdkHttpOperationExample,
SdkHttpParameter,
SdkHttpParameterExampleValue,
SdkHttpResponse,
SdkHttpResponseExampleValue,
SdkModelExampleValue,
SdkNullExampleValue,
Expand All @@ -34,7 +32,6 @@ import {
InputUnknownExampleValue,
OperationResponseExample,
} from "../type/input-examples.js";
import { InputParameter } from "../type/input-parameter.js";
import {
InputArrayType,
InputDictionaryType,
Expand All @@ -43,14 +40,12 @@ import {
InputPrimitiveType,
InputUnionType,
} from "../type/input-type.js";
import { OperationResponse } from "../type/operation-response.js";
import { fromSdkHttpOperationResponse } from "./operation-converter.js";
import { fromSdkType } from "./type-converter.js";

export function fromSdkHttpExamples(
sdkContext: CSharpEmitterContext,
examples: SdkHttpOperationExample[],
parameterMap: Map<SdkHttpParameter, InputParameter>,
responseMap: Map<SdkHttpResponse, OperationResponse>,
): InputHttpOperationExample[] {
return examples.map((example) => fromSdkHttpExample(example));

Expand All @@ -61,36 +56,26 @@ export function fromSdkHttpExamples(
description: example.description,
filePath: example.filePath,
parameters: example.parameters.map((p) => fromSdkParameterExample(p)),
responses: fromSdkOperationResponses(example.responses),
responses: example.responses.map((r) => fromSdkOperationResponse(r)),
};
}

function fromSdkParameterExample(
parameter: SdkHttpParameterExampleValue,
): InputParameterExampleValue {
return {
parameter: parameterMap.get(parameter.parameter)!,
parameter: sdkContext.__typeCache.parameters.get(parameter.parameter)!,
value: fromSdkExample(parameter.value),
};
}

function fromSdkOperationResponses(
responses: SdkHttpResponseExampleValue[],
): OperationResponseExample[] {
const result: OperationResponseExample[] = [];
for (const response of responses) {
result.push(fromSdkOperationResponse(response));
}
return result;
}

function fromSdkOperationResponse(
response: SdkHttpResponseExampleValue,
responseValue: SdkHttpResponseExampleValue,
): OperationResponseExample {
return {
response: responseMap.get(response.response)!,
statusCode: response.statusCode,
bodyValue: response.bodyValue ? fromSdkExample(response.bodyValue) : undefined,
response: fromSdkHttpOperationResponse(sdkContext, responseValue.response),
statusCode: responseValue.statusCode,
bodyValue: responseValue.bodyValue ? fromSdkExample(responseValue.bodyValue) : undefined,
};
}

Expand Down
72 changes: 47 additions & 25 deletions packages/http-client-csharp/emitter/src/lib/operation-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ export function fromSdkServiceMethod(
generateConvenience = false;
}

const parameterMap = fromSdkOperationParameters(sdkContext, method.operation, rootApiVersions);
const responseMap = fromSdkHttpOperationResponses(sdkContext, method.operation.responses);
return {
Name: method.name,
ResourceName:
Expand All @@ -69,8 +67,8 @@ export function fromSdkServiceMethod(
Summary: method.summary,
Doc: method.doc,
Accessibility: method.access,
Parameters: [...parameterMap.values()],
Responses: [...responseMap.values()],
Parameters: fromSdkOperationParameters(sdkContext, method.operation, rootApiVersions),
Responses: fromSdkHttpOperationResponses(sdkContext, method.operation.responses),
HttpMethod: parseHttpRequestMethod(method.operation.verb),
RequestBodyMediaType: getBodyMediaType(method.operation.bodyParam?.type),
Uri: uri,
Expand All @@ -85,7 +83,7 @@ export function fromSdkServiceMethod(
CrossLanguageDefinitionId: method.crossLanguageDefinitionId,
Decorators: method.decorators,
Examples: method.operation.examples
? fromSdkHttpExamples(sdkContext, method.operation.examples, parameterMap, responseMap)
? fromSdkHttpExamples(sdkContext, method.operation.examples)
: undefined,
};
}
Expand Down Expand Up @@ -138,8 +136,8 @@ function fromSdkOperationParameters(
sdkContext: CSharpEmitterContext,
operation: SdkHttpOperation,
rootApiVersions: string[],
): Map<SdkHttpParameter, InputParameter> {
const parameters = new Map<SdkHttpParameter, InputParameter>();
): InputParameter[] {
const parameters: InputParameter[] = [];
for (const p of operation.parameters) {
if (p.kind === "cookie") {
sdkContext.logger.reportDiagnostic({
Expand All @@ -150,7 +148,7 @@ function fromSdkOperationParameters(
return parameters;
}
const param = fromSdkHttpOperationParameter(sdkContext, p, rootApiVersions);
parameters.set(p, param);
parameters.push(param);
}

if (operation.bodyParam) {
Expand All @@ -159,36 +157,41 @@ function fromSdkOperationParameters(
operation.bodyParam,
rootApiVersions,
);
parameters.set(operation.bodyParam, bodyParam);
parameters.push(bodyParam);
}
return parameters;
}

function fromSdkHttpOperationParameter(
export function fromSdkHttpOperationParameter(
sdkContext: CSharpEmitterContext,
p: SdkPathParameter | SdkQueryParameter | SdkHeaderParameter | SdkBodyParameter,
rootApiVersions: string[],
): InputParameter {
let retVar = sdkContext.__typeCache.parameters.get(p);
if (retVar) {
return retVar;
}

const isContentType =
p.kind === "header" && p.serializedName.toLocaleLowerCase() === "content-type";
const parameterType = fromSdkType(sdkContext, p.type);
const format = p.kind === "header" || p.kind === "query" ? p.collectionFormat : undefined;
const serializedName = p.kind !== "body" ? p.serializedName : p.name;

// TO-DO: In addition to checking if a path parameter is exploded, we should consider capturing the delimiter for
// TODO: In addition to checking if a path parameter is exploded, we should consider capturing the delimiter for
// any path expansion to ensure the parameter values are delimited correctly during serialization.
// https://github.com/microsoft/typespec/issues/5561
const explode = isExplodedParameter(p);

return {
retVar = {
Name: p.name,
NameInRequest: p.kind === "header" ? normalizeHeaderName(serializedName) : serializedName,
Summary: p.summary,
Doc: p.doc,
Type: parameterType,
Location: getParameterLocation(p),
IsApiVersion:
p.name.toLocaleLowerCase() === "apiversion" || p.name.toLocaleLowerCase() === "api-version",
p.name.toLocaleLowerCase() === "apiversion" || p.name.toLocaleLowerCase() === "api-version", // TODO -- we should use `isApiVersionParam` instead
IsContentType: isContentType,
IsEndpoint: false,
Explode: explode,
Expand All @@ -198,7 +201,10 @@ function fromSdkHttpOperationParameter(
DefaultValue: getParameterDefaultValue(sdkContext, p.clientDefaultValue, parameterType),
Decorators: p.decorators,
SkipUrlEncoding: p.kind === "path" ? p.allowReserved : false,
} as InputParameter;
};

sdkContext.__typeCache.updateSdkParameterReferences(p, retVar);
return retVar;
}

function loadLongRunningOperation(
Expand Down Expand Up @@ -227,22 +233,38 @@ function loadLongRunningOperation(
function fromSdkHttpOperationResponses(
sdkContext: CSharpEmitterContext,
operationResponses: SdkHttpResponse[],
): Map<SdkHttpResponse, OperationResponse> {
const responses = new Map<SdkHttpResponse, OperationResponse>();
): OperationResponse[] {
const responses: OperationResponse[] = [];
for (const r of operationResponses) {
const range = r.statusCodes;
responses.set(r, {
StatusCodes: toStatusCodesArray(range),
BodyType: r.type ? fromSdkType(sdkContext, r.type) : undefined,
BodyMediaType: BodyMediaType.Json,
Headers: fromSdkServiceResponseHeaders(sdkContext, r.headers),
IsErrorResponse: r.type !== undefined && isErrorModel(sdkContext.program, r.type.__raw!),
ContentTypes: r.contentTypes,
});
responses.push(fromSdkHttpOperationResponse(sdkContext, r));
}
return responses;
}

export function fromSdkHttpOperationResponse(
sdkContext: CSharpEmitterContext,
sdkResponse: SdkHttpResponse,
): OperationResponse {
let retVar = sdkContext.__typeCache.responses.get(sdkResponse);
if (retVar) {
return retVar;
}

const range = sdkResponse.statusCodes;
retVar = {
StatusCodes: toStatusCodesArray(range),
BodyType: sdkResponse.type ? fromSdkType(sdkContext, sdkResponse.type) : undefined,
BodyMediaType: BodyMediaType.Json,
Headers: fromSdkServiceResponseHeaders(sdkContext, sdkResponse.headers),
IsErrorResponse:
sdkResponse.type !== undefined && isErrorModel(sdkContext.program, sdkResponse.type.__raw!),
ContentTypes: sdkResponse.contentTypes,
};

sdkContext.__typeCache.updateSdkResponseReferences(sdkResponse, retVar);
return retVar;
}

function fromSdkServiceResponseHeaders(
sdkContext: CSharpEmitterContext,
headers: SdkServiceResponseHeader[],
Expand Down
30 changes: 7 additions & 23 deletions packages/http-client-csharp/emitter/src/lib/type-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ export function fromSdkType(
sdkType: SdkType,
literalTypeContext?: LiteralTypeContext,
): InputType {
if (sdkContext.__typeCache.types.has(sdkType)) {
return sdkContext.__typeCache.types.get(sdkType)!;
let retVar = sdkContext.__typeCache.types.get(sdkType);
if (retVar) {
return retVar;
}

let retVar: InputType;
switch (sdkType.kind) {
case "nullable":
const inputType = fromSdkType(sdkContext, sdkType.type);
Expand Down Expand Up @@ -104,26 +104,10 @@ export function fromSdkType(
break;
}

updateSdkTypeReferences(sdkContext, sdkType, retVar);
sdkContext.__typeCache.updateSdkTypeReferences(sdkType, retVar);
return retVar;
}

function updateTypeCache(sdkContext: CSharpEmitterContext, typeName: string, type: InputType) {
if (type.kind === "model") {
sdkContext.__typeCache.models.set(typeName, type);
} else if (type.kind === "enum") {
sdkContext.__typeCache.enums.set(typeName, type);
}
}

function updateSdkTypeReferences(
sdkContext: CSharpEmitterContext,
sdkType: SdkType,
inputType: InputType,
) {
sdkContext.__typeCache.types.set(sdkType, inputType);
}

export function fromSdkModelType(
sdkContext: CSharpEmitterContext,
modelType: SdkModelType,
Expand All @@ -145,7 +129,7 @@ export function fromSdkModelType(
decorators: modelType.decorators,
} as InputModelType;

updateTypeCache(sdkContext, modelTypeName, inputModelType);
sdkContext.__typeCache.updateTypeCache(modelTypeName, inputModelType);

inputModelType.additionalProperties = modelType.additionalProperties
? fromSdkType(sdkContext, modelType.additionalProperties)
Expand Down Expand Up @@ -250,7 +234,7 @@ export function fromSdkEnumType(
decorators: enumType.decorators,
};
if (addToCollection) {
updateTypeCache(sdkContext, enumName, inputEnumType);
sdkContext.__typeCache.updateTypeCache(enumName, inputEnumType);
}
for (const v of enumType.values) {
values.push(fromSdkEnumValueType(sdkContext, v));
Expand Down Expand Up @@ -373,7 +357,7 @@ function fromSdkConstantType(
decorators: constantType.decorators,
};

updateTypeCache(sdkContext, enumName, enumType);
sdkContext.__typeCache.updateTypeCache(enumName, enumType);

values.push({
kind: "enumvalue",
Expand Down
2 changes: 0 additions & 2 deletions packages/http-client-csharp/emitter/src/lib/typespec-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export function resolveServers(
Type: inputType,
Location: RequestLocation.Uri,
IsApiVersion: name.toLowerCase() === "apiversion" || name.toLowerCase() === "api-version",
IsResourceParameter: false,
IsContentType: false,
IsRequired: true,
IsEndpoint: isEndpoint,
Expand All @@ -79,7 +78,6 @@ export function resolveServers(
},
Location: RequestLocation.Uri,
IsApiVersion: false,
IsResourceParameter: false,
IsContentType: false,
IsRequired: true,
IsEndpoint: true,
Expand Down
Loading
Loading