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

add a cache for parameters/responses/operations #6030

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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

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
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
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
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
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