Skip to content

Align the property types and refine the caches for properties #7340

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Align the property types and refine the caches for properties #7340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
36c929b
update
ArcturusZhang May 14, 2025
850d63f
a few updates
ArcturusZhang May 14, 2025
141f152
wip
ArcturusZhang May 14, 2025
b2c230c
wip
ArcturusZhang May 14, 2025
03f3082
API changed now we need to fix all the usage to comply
ArcturusZhang May 15, 2025
ad116a4
fix C# side issues
ArcturusZhang May 15, 2025
3f8fc1d
fix test cases
ArcturusZhang May 15, 2025
32b741f
Merge branch 'main' into align-tcgc-property-types
ArcturusZhang May 15, 2025
0e3ce10
Merge remote-tracking branch 'origin/main' into align-tcgc-property-t…
ArcturusZhang May 19, 2025
f0820c1
regen
ArcturusZhang May 19, 2025
c464df5
Merge remote-tracking branch 'origin/main' into align-tcgc-property-t…
ArcturusZhang May 20, 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
104 changes: 81 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 @@ -11,9 +11,11 @@ import {
SdkDurationType,
SdkEnumType,
SdkEnumValueType,
SdkHttpParameter,
SdkHeaderParameter,
SdkModelPropertyType,
SdkModelType,
SdkPathParameter,
SdkQueryParameter,
SdkTupleType,
SdkType,
SdkUnionType,
Expand All @@ -31,10 +33,14 @@ import {
InputDurationType,
InputEnumType,
InputEnumValueType,
InputHeaderParameter,
InputLiteralType,
InputModelProperty,
InputModelType,
InputPathParameter,
InputPrimitiveType,
InputProperty,
InputQueryParameter,
InputType,
InputUnionType,
} from "../type/input-type.js";
Expand Down Expand Up @@ -133,25 +139,24 @@ export function fromSdkModelType(
? fromSdkType(sdkContext, modelType.additionalProperties)
: undefined;

const propertiesDict = new Map<SdkModelPropertyType, InputModelProperty>();
const properties: InputProperty[] = [];
for (const property of modelType.properties) {
const ourProperty = fromSdkModelProperty(sdkContext, property);

if (!ourProperty) {
continue;
if (ourProperty) {
properties.push(ourProperty);
}
propertiesDict.set(property, ourProperty);
}

inputModelType.discriminatorProperty = modelType.discriminatorProperty
? propertiesDict.get(modelType.discriminatorProperty)
? fromSdkModelProperty(sdkContext, modelType.discriminatorProperty)
: undefined;

inputModelType.baseModel = modelType.baseModel
? fromSdkModelType(sdkContext, modelType.baseModel)
: undefined;

inputModelType.properties = Array.from(propertiesDict.values()).flat();
inputModelType.properties = properties;

if (modelType.discriminatedSubtypes) {
const discriminatedSubtypes: Record<string, InputModelType> = {};
Expand All @@ -167,42 +172,95 @@ export function fromSdkModelType(

function fromSdkModelProperty(
sdkContext: CSharpEmitterContext,
property: SdkModelPropertyType,
): InputModelProperty | undefined {
switch (property.kind) {
sdkProperty: SdkModelPropertyType,
): InputProperty | undefined {
// TODO -- this returns undefined because some properties we do not support yet.
let property = sdkContext.__typeCache.properties.get(sdkProperty) as InputProperty | undefined;
if (property) {
return property;
}
switch (sdkProperty.kind) {
case "property":
return fromSdkBodyModelProperty(sdkContext, property);
property = fromSdkBodyModelProperty(sdkContext, sdkProperty);
break;
case "header":
property = fromSdkHeaderParameter(sdkContext, sdkProperty);
break;
case "query":
property = fromSdkQueryParameter(sdkContext, sdkProperty);
break;
case "path":
return fromSdkHttpParameterModelProperty(sdkContext, property);
default:
return undefined;
property = fromSdkPathParameter(sdkContext, sdkProperty);
break;
}

if (property) {
sdkContext.__typeCache.updateSdkPropertyReferences(sdkProperty, property);
}

return property;
}

function fromSdkHttpParameterModelProperty(
function fromSdkHeaderParameter(
sdkContext: CSharpEmitterContext,
property: SdkHttpParameter,
): InputModelProperty {
const targetType = property.type;
property: SdkHeaderParameter,
): InputHeaderParameter {
return {
kind: property.kind,
name: property.name,
serializedName: property.serializedName,
summary: property.summary,
doc: property.doc,
type: fromSdkType(sdkContext, property.type),
optional: property.optional,
readOnly: isReadOnly(property),
decorators: property.decorators,
crossLanguageDefinitionId: property.crossLanguageDefinitionId,
collectionFormat: property.collectionFormat,
correspondingMethodParams: [], // TODO - this should be a ref of other properties
};
}

const modelHeaderProperty: InputModelProperty = {
function fromSdkQueryParameter(
sdkContext: CSharpEmitterContext,
property: SdkQueryParameter,
): InputQueryParameter {
return {
kind: property.kind,
name: property.name,
serializedName: property.serializedName,
summary: property.summary,
doc: property.doc,
type: fromSdkType(sdkContext, targetType),
type: fromSdkType(sdkContext, property.type),
optional: property.optional,
readOnly: isReadOnly(property),
decorators: property.decorators,
crossLanguageDefinitionId: property.crossLanguageDefinitionId,
discriminator: false,
flatten: false,
correspondingMethodParams: [], // TODO - this should be a ref of other properties
explode: property.explode,
};
}

return modelHeaderProperty;
function fromSdkPathParameter(
sdkContext: CSharpEmitterContext,
property: SdkPathParameter,
): InputPathParameter {
return {
kind: property.kind,
name: property.name,
serializedName: property.serializedName,
summary: property.summary,
doc: property.doc,
type: fromSdkType(sdkContext, property.type),
optional: property.optional,
readOnly: isReadOnly(property),
decorators: property.decorators,
crossLanguageDefinitionId: property.crossLanguageDefinitionId,
explode: property.explode,
style: property.style,
allowReserved: property.allowReserved,
correspondingMethodParams: [], // TODO - this should be a ref of other properties
};
}

function fromSdkBodyModelProperty(
Expand Down
8 changes: 4 additions & 4 deletions packages/http-client-csharp/emitter/src/sdk-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import {
InputClient,
InputEnumType,
InputLiteralType,
InputModelProperty,
InputModelType,
InputProperty,
InputType,
} from "./type/input-type.js";
import { OperationResponse } from "./type/operation-response.js";
Expand Down Expand Up @@ -54,7 +54,7 @@ export function createCSharpEmitterContext<

class SdkTypeCache {
clients: Map<SdkClientType<SdkHttpOperation>, InputClient>;
properties: Map<SdkModelPropertyType, InputParameter | InputModelProperty>;
properties: Map<SdkModelPropertyType, InputParameter | InputProperty>; // TODO -- in the near future, we should replace `InputParameter` with those `InputQueryParameter`, etc.
responses: Map<SdkHttpResponse, OperationResponse>;
types: Map<SdkType, InputType>;
models: Map<string, InputModelType>;
Expand All @@ -64,7 +64,7 @@ class SdkTypeCache {

constructor() {
this.clients = new Map<SdkClientType<SdkHttpOperation>, InputClient>();
this.properties = new Map<SdkModelPropertyType, InputParameter | InputModelProperty>();
this.properties = new Map<SdkModelPropertyType, InputParameter | InputProperty>();
this.responses = new Map<SdkHttpResponse, OperationResponse>();
this.types = new Map<SdkType, InputType>();
this.models = new Map<string, InputModelType>();
Expand All @@ -80,7 +80,7 @@ class SdkTypeCache {

updateSdkPropertyReferences(
sdkProperty: SdkModelPropertyType,
inputProperty: InputParameter | InputModelProperty,
inputProperty: InputParameter | InputProperty,
) {
this.properties.set(sdkProperty, inputProperty);
this.crossLanguageDefinitionIds.set(sdkProperty.crossLanguageDefinitionId, sdkProperty.__raw);
Expand Down
68 changes: 57 additions & 11 deletions packages/http-client-csharp/emitter/src/type/input-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

import {
AccessFlags,
CollectionFormat,
DecoratorInfo,
SdkBuiltInKinds,
SdkModelPropertyType,
SerializationOptions,
UsageFlags,
} from "@azure-tools/typespec-client-generator-core";
Expand Down Expand Up @@ -121,7 +121,7 @@ export function isInputUnionType(type: InputType): type is InputUnionType {
*/
export interface InputModelType extends InputTypeBase {
kind: "model";
properties: InputModelProperty[];
properties: InputProperty[];
name: string;
crossLanguageDefinitionId: string;
access?: AccessFlags;
Expand All @@ -130,26 +130,72 @@ export interface InputModelType extends InputTypeBase {
additionalProperties?: InputType;
discriminatorValue?: string;
discriminatedSubtypes?: Record<string, InputModelType>;
discriminatorProperty?: InputModelProperty;
discriminatorProperty?: InputProperty;
baseModel?: InputModelType;
serializationOptions: SerializationOptions;
}

export interface InputModelProperty extends InputTypeBase {
kind: SdkModelPropertyType["kind"];
name: string;
serializedName: string;
export interface InputModelPropertyTypeBase extends DecoratedType {
type: InputType;
name: string;
doc?: string;
summary?: string;
// apiVersions: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove commented code

// onClient: boolean;
// clientDefaultValue?: unknown;
// isApiVersionParam: boolean;
optional: boolean;
crossLanguageDefinitionId: string;
readOnly: boolean;
access?: AccessFlags;
}

export interface InputModelProperty extends InputModelPropertyTypeBase {
kind: "property";
discriminator: boolean;
crossLanguageDefinitionId: string;
serializedName: string;
serializationOptions: SerializationOptions;
flatten: boolean;
serializationOptions?: SerializationOptions;
}

export function isInputModelType(type: InputType): type is InputModelType {
return type.kind === "model";
export type InputProperty = InputModelProperty | InputHttpParameter;

export type InputHttpParameter =
| InputQueryParameter
| InputPathParameter
| InputHeaderParameter
| InputBodyParameter;

export interface InputQueryParameter extends InputModelPropertyTypeBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this type extend from InputModelProperty? It won't always be on a model, will it?

kind: "query";
collectionFormat?: CollectionFormat;
serializedName: string;
correspondingMethodParams: InputProperty[];
explode: boolean;
}

export interface InputPathParameter extends InputModelPropertyTypeBase {
kind: "path";
explode: boolean;
style: "simple" | "label" | "matrix" | "fragment" | "path";
allowReserved: boolean;
serializedName: string;
correspondingMethodParams: InputProperty[];
}

export interface InputHeaderParameter extends InputModelPropertyTypeBase {
kind: "header";
collectionFormat?: CollectionFormat;
serializedName: string;
correspondingMethodParams: InputProperty[];
}

export interface InputBodyParameter extends InputModelPropertyTypeBase {
kind: "body";
serializedName: string;
contentTypes: string[];
defaultContentType: string;
correspondingMethodParams: InputProperty[];
}

export interface InputEnumType extends InputTypeBase {
Expand Down
12 changes: 9 additions & 3 deletions packages/http-client-csharp/emitter/test/Unit/model-type.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ op test(@body input: Pet): Pet;
const discriminatorProperty = petModel?.properties.find(
(p) => p === petModel?.discriminatorProperty,
);
strictEqual(discriminatorProperty?.name, "kind");
ok(discriminatorProperty);
strictEqual(discriminatorProperty.kind, "property");
strictEqual(discriminatorProperty.name, "kind");
strictEqual(discriminatorProperty.serializedName, "kind");
strictEqual(discriminatorProperty.type.kind, "string");
strictEqual(discriminatorProperty.optional, false);
Expand Down Expand Up @@ -140,7 +142,9 @@ op test(@body input: Pet): Pet;
strictEqual("kind", pet?.discriminatorProperty?.name);
// assert we have a property corresponding to the discriminator property above on the base model
const discriminatorProperty = pet?.properties.find((p) => p === pet?.discriminatorProperty);
strictEqual(discriminatorProperty?.name, "kind");
ok(discriminatorProperty);
strictEqual(discriminatorProperty.kind, "property");
strictEqual(discriminatorProperty.name, "kind");
strictEqual(discriminatorProperty.serializedName, "kind");
strictEqual(discriminatorProperty.doc, "The kind of the pet");
strictEqual(discriminatorProperty.type.kind, "enum");
Expand Down Expand Up @@ -232,7 +236,9 @@ op test(@body input: Pet): Pet;
strictEqual("kind", pet?.discriminatorProperty?.name);
// assert we have a property corresponding to the discriminator property above on the base model
const discriminatorProperty = pet?.properties.find((p) => p === pet?.discriminatorProperty);
strictEqual(discriminatorProperty?.name, "kind");
ok(discriminatorProperty);
strictEqual(discriminatorProperty.kind, "property");
strictEqual(discriminatorProperty.name, "kind");
strictEqual(discriminatorProperty.serializedName, "kind");
strictEqual(discriminatorProperty.doc, "The kind of the pet");
strictEqual(discriminatorProperty.type.kind, "enum");
Expand Down
Loading
Loading