-
Notifications
You must be signed in to change notification settings - Fork 85
tsp, handle spread behavior change from TCGC #2934
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
be8c404
public override spread
weidongxu-microsoft 5379521
re-generate test code
actions-user f01fa49
use PublicSpread
weidongxu-microsoft 878412e
Merge branch 'main' into tsp_try-fix-spread
weidongxu-microsoft 0df9583
format
weidongxu-microsoft adbe878
update comment
weidongxu-microsoft 9cbf0e7
use includes, improve comment
weidongxu-microsoft d44c3a7
bump to 0.20.1
weidongxu-microsoft c8b38e3
Merge branch 'main' into tsp_try-fix-spread
weidongxu-microsoft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,7 +67,6 @@ import { | |
| SdkServiceMethod, | ||
| SdkType, | ||
| SdkUnionType, | ||
| UsageFlags, | ||
| createSdkContext, | ||
| getAllModels, | ||
| getClientType, | ||
|
|
@@ -107,10 +106,7 @@ import { | |
| getHeaderFieldName, | ||
| getPathParamName, | ||
| getQueryParamName, | ||
| isBody, | ||
| isBodyRoot, | ||
| isHeader, | ||
| isMultipartBodyProperty, | ||
| isPathParam, | ||
| isQueryParam, | ||
| } from "@typespec/http"; | ||
|
|
@@ -455,6 +451,7 @@ export class CodeModelBuilder { | |
| schema instanceof ConstantSchema | ||
| ) { | ||
| const schemaUsage: SchemaContext[] | undefined = schema.usage; | ||
|
|
||
| // Public override Internal | ||
| if (schemaUsage?.includes(SchemaContext.Public)) { | ||
| const index = schemaUsage.indexOf(SchemaContext.Internal); | ||
|
|
@@ -463,11 +460,17 @@ export class CodeModelBuilder { | |
| } | ||
| } | ||
|
|
||
| // Internal on Anonymous | ||
| if (schemaUsage?.includes(SchemaContext.Anonymous)) { | ||
| const index = schemaUsage.indexOf(SchemaContext.Internal); | ||
| if (index < 0) { | ||
| schemaUsage.push(SchemaContext.Internal); | ||
| // Internal on PublicSpread, but Public takes precedence | ||
| if (schemaUsage?.includes(SchemaContext.PublicSpread)) { | ||
| // remove PublicSpread as it now served its purpose | ||
| schemaUsage.splice(schemaUsage.indexOf(SchemaContext.PublicSpread), 1); | ||
|
|
||
| // Public would override PublicSpread, hence do nothing if this schema is Public | ||
| if (!schemaUsage?.includes(SchemaContext.Public)) { | ||
| // set the model as Internal, so that it is not exposed to user | ||
| if (!schemaUsage.includes(SchemaContext.Internal)) { | ||
| schemaUsage.push(SchemaContext.Internal); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1302,119 +1305,138 @@ export class CodeModelBuilder { | |
| }); | ||
| op.addParameter(parameter); | ||
|
|
||
| const jsonMergePatch = operationIsJsonMergePatch(sdkHttpOperation); | ||
|
|
||
| const schemaIsPublicBeforeProcess = | ||
| schema instanceof ObjectSchema && (schema as SchemaUsage).usage?.includes(SchemaContext.Public); | ||
|
|
||
| this.trackSchemaUsage(schema, { usage: [SchemaContext.Input] }); | ||
|
|
||
| if (op.convenienceApi) { | ||
| // model/schema does not need to be Public or Internal, if it is not to be used in convenience API | ||
| this.trackSchemaUsage(schema, { usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public] }); | ||
| } | ||
|
|
||
| if (operationIsJsonMergePatch(sdkHttpOperation)) { | ||
| if (jsonMergePatch) { | ||
| this.trackSchemaUsage(schema, { usage: [SchemaContext.JsonMergePatch] }); | ||
| } | ||
| if (op.convenienceApi && operationIsMultipart(sdkHttpOperation)) { | ||
| this.trackSchemaUsage(schema, { serializationFormats: [KnownMediaType.Multipart] }); | ||
| } | ||
|
|
||
| // Implicit body parameter would have usage flag: UsageFlags.Spread, for this case we need to do body parameter flatten | ||
| const bodyParameterFlatten = sdkType.kind === "model" && sdkType.usage & UsageFlags.Spread && !this.isArm(); | ||
|
|
||
| if (schema instanceof ObjectSchema && bodyParameterFlatten) { | ||
| // flatten body parameter | ||
| const parameters = sdkHttpOperation.parameters; | ||
| const bodyParameter = sdkHttpOperation.bodyParam; | ||
| // name the schema for documentation | ||
| schema.language.default.name = pascalCase(op.language.default.name) + "Request"; | ||
|
|
||
| if (!parameter.language.default.name) { | ||
| // name the parameter for documentation | ||
| parameter.language.default.name = "request"; | ||
| } | ||
| if (op.convenienceApi) { | ||
| // Explicit body parameter @body or @bodyRoot would result to the existance of rawHttpOperation.parameters.body.property | ||
| // Implicit body parameter would result to rawHttpOperation.parameters.body.property be undefined | ||
| // see https://typespec.io/docs/libraries/http/cheat-sheet#data-types | ||
| const bodyParameterFlatten = | ||
| schema instanceof ObjectSchema && | ||
| sdkType.kind === "model" && | ||
| !rawHttpOperation.parameters.body?.property && | ||
| !this.isArm(); | ||
|
|
||
| if (schema instanceof ObjectSchema && bodyParameterFlatten) { | ||
| // flatten body parameter | ||
| const parameters = sdkHttpOperation.parameters; | ||
| const bodyParameter = sdkHttpOperation.bodyParam; | ||
|
|
||
| if (!parameter.language.default.name) { | ||
| // name the parameter for documentation | ||
| parameter.language.default.name = "request"; | ||
| } | ||
|
|
||
| if (operationIsJsonMergePatch(sdkHttpOperation)) { | ||
| // skip model flatten, if "application/merge-patch+json" | ||
| schema.language.default.name = pascalCase(op.language.default.name) + "PatchRequest"; | ||
| return; | ||
| } | ||
|
Comment on lines
-1334
to
-1338
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and - schema.language.default.name = pascalCase(op.language.default.name) + "Request";These change does not relate to the issue. But we should use name from TCGC in almost all cases. This is the cause of the diff of generated code in this PR. |
||
| if (jsonMergePatch) { | ||
| // skip model flatten, if "application/merge-patch+json" | ||
| if (sdkType.isGeneratedName) { | ||
| schema.language.default.name = pascalCase(op.language.default.name) + "PatchRequest"; | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| this.trackSchemaUsage(schema, { usage: [SchemaContext.Anonymous] }); | ||
| const schemaUsage = (schema as SchemaUsage).usage; | ||
| if (!schemaIsPublicBeforeProcess && schemaUsage?.includes(SchemaContext.Public)) { | ||
| // Public added in this op, change it to PublicSpread | ||
| // This means that if this op would originally add Public to this schema, it adds PublicSpread instead | ||
| schemaUsage?.splice(schemaUsage?.indexOf(SchemaContext.Public), 1); | ||
| this.trackSchemaUsage(schema, { usage: [SchemaContext.PublicSpread] }); | ||
| } | ||
|
|
||
| if (op.convenienceApi && op.parameters) { | ||
| op.convenienceApi.requests = []; | ||
| const request = new Request({ | ||
| protocol: op.requests![0].protocol, | ||
| }); | ||
| request.parameters = []; | ||
| op.convenienceApi.requests.push(request); | ||
| if (op.convenienceApi && op.parameters) { | ||
| op.convenienceApi.requests = []; | ||
| const request = new Request({ | ||
| protocol: op.requests![0].protocol, | ||
| }); | ||
| request.parameters = []; | ||
| op.convenienceApi.requests.push(request); | ||
|
|
||
| // header/query/path params | ||
| for (const opParameter of parameters) { | ||
| this.addParameterOrBodyPropertyToCodeModelRequest(opParameter, op, request, schema, parameter); | ||
| } | ||
| // body param | ||
| if (bodyParameter) { | ||
| if (bodyParameter.type.kind === "model") { | ||
| for (const bodyProperty of bodyParameter.type.properties) { | ||
| if (bodyProperty.kind === "property") { | ||
| this.addParameterOrBodyPropertyToCodeModelRequest(bodyProperty, op, request, schema, parameter); | ||
| // header/query/path params | ||
| for (const opParameter of parameters) { | ||
| this.addParameterOrBodyPropertyToCodeModelRequest(opParameter, op, request, schema, parameter); | ||
| } | ||
| // body param | ||
| if (bodyParameter) { | ||
| if (bodyParameter.type.kind === "model") { | ||
| for (const bodyProperty of bodyParameter.type.properties) { | ||
| if (bodyProperty.kind === "property") { | ||
| this.addParameterOrBodyPropertyToCodeModelRequest(bodyProperty, op, request, schema, parameter); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| request.signatureParameters = request.parameters; | ||
|
|
||
| if (request.signatureParameters.length > 6) { | ||
| // create an option bag | ||
| const name = op.language.default.name + "Options"; | ||
| const namespace = getNamespace(rawHttpOperation.operation); | ||
| // option bag schema | ||
| const optionBagSchema = this.codeModel.schemas.add( | ||
| new GroupSchema(name, `Options for ${op.language.default.name} API`, { | ||
| language: { | ||
| default: { | ||
| namespace: namespace, | ||
| }, | ||
| java: { | ||
| namespace: this.getJavaNamespace(namespace), | ||
| request.signatureParameters = request.parameters; | ||
|
|
||
| if (request.signatureParameters.length > 6) { | ||
| // create an option bag | ||
| const name = op.language.default.name + "Options"; | ||
| const namespace = getNamespace(rawHttpOperation.operation); | ||
| // option bag schema | ||
| const optionBagSchema = this.codeModel.schemas.add( | ||
| new GroupSchema(name, `Options for ${op.language.default.name} API`, { | ||
| language: { | ||
| default: { | ||
| namespace: namespace, | ||
| }, | ||
| java: { | ||
| namespace: this.getJavaNamespace(namespace), | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| ); | ||
| request.parameters.forEach((it) => { | ||
| optionBagSchema.add( | ||
| new GroupProperty(it.language.default.name, it.language.default.description, it.schema, { | ||
| originalParameter: [it], | ||
| summary: it.summary, | ||
| required: it.required, | ||
| nullable: it.nullable, | ||
| readOnly: false, | ||
| serializedName: it.language.default.serializedName, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| this.trackSchemaUsage(optionBagSchema, { usage: [SchemaContext.Input] }); | ||
| if (op.convenienceApi) { | ||
| this.trackSchemaUsage(optionBagSchema, { | ||
| usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public], | ||
| request.parameters.forEach((it) => { | ||
| optionBagSchema.add( | ||
| new GroupProperty(it.language.default.name, it.language.default.description, it.schema, { | ||
| originalParameter: [it], | ||
| summary: it.summary, | ||
| required: it.required, | ||
| nullable: it.nullable, | ||
| readOnly: false, | ||
| serializedName: it.language.default.serializedName, | ||
| }), | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| // option bag parameter | ||
| const optionBagParameter = new Parameter( | ||
| "options", | ||
| optionBagSchema.language.default.description, | ||
| optionBagSchema, | ||
| { | ||
| implementation: ImplementationLocation.Method, | ||
| required: true, | ||
| nullable: false, | ||
| }, | ||
| ); | ||
| this.trackSchemaUsage(optionBagSchema, { usage: [SchemaContext.Input] }); | ||
| if (op.convenienceApi) { | ||
| this.trackSchemaUsage(optionBagSchema, { | ||
| usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public], | ||
| }); | ||
| } | ||
|
|
||
| // option bag parameter | ||
| const optionBagParameter = new Parameter( | ||
| "options", | ||
| optionBagSchema.language.default.description, | ||
| optionBagSchema, | ||
| { | ||
| implementation: ImplementationLocation.Method, | ||
| required: true, | ||
| nullable: false, | ||
| }, | ||
| ); | ||
|
|
||
| request.signatureParameters = [optionBagParameter]; | ||
| request.parameters.forEach((it) => (it.groupedBy = optionBagParameter)); | ||
| request.parameters.push(optionBagParameter); | ||
| request.signatureParameters = [optionBagParameter]; | ||
| request.parameters.forEach((it) => (it.groupedBy = optionBagParameter)); | ||
| request.parameters.push(optionBagParameter); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2229,24 +2251,6 @@ export class CodeModelBuilder { | |
| } | ||
| } | ||
|
|
||
| private getParameterLocation(target: ModelProperty): ParameterLocation | "BodyProperty" { | ||
| if (isHeader(this.program, target)) { | ||
| return ParameterLocation.Header; | ||
| } else if (isQueryParam(this.program, target)) { | ||
| return ParameterLocation.Query; | ||
| } else if (isPathParam(this.program, target)) { | ||
| return ParameterLocation.Path; | ||
| } else if ( | ||
| isBody(this.program, target) || | ||
| isBodyRoot(this.program, target) || | ||
| isMultipartBodyProperty(this.program, target) | ||
| ) { | ||
| return ParameterLocation.Body; | ||
| } else { | ||
| return "BodyProperty"; | ||
| } | ||
| } | ||
|
|
||
| private isReadOnly(target: SdkModelPropertyType): boolean { | ||
| const segment = target.__raw ? getSegment(this.program, target.__raw) !== undefined : false; | ||
| if (segment) { | ||
|
|
@@ -2516,10 +2520,21 @@ export class CodeModelBuilder { | |
| }; | ||
|
|
||
| // Exclude context that not to be propagated | ||
| const updatedSchemaUsage = (schema as SchemaUsage).usage?.filter( | ||
| (it) => it !== SchemaContext.Paged && it !== SchemaContext.PublicSpread, | ||
| ); | ||
| const indexSpread = (schema as SchemaUsage).usage?.indexOf(SchemaContext.PublicSpread); | ||
| if ( | ||
| updatedSchemaUsage && | ||
| indexSpread && | ||
| indexSpread >= 0 && | ||
| !(schema as SchemaUsage).usage?.includes(SchemaContext.Public) | ||
| ) { | ||
| // Propagate Public, if schema is PublicSpread | ||
| updatedSchemaUsage.push(SchemaContext.Public); | ||
| } | ||
| const schemaUsage = { | ||
| usage: (schema as SchemaUsage).usage?.filter( | ||
| (it) => it !== SchemaContext.Paged && it !== SchemaContext.Anonymous, | ||
| ), | ||
| usage: updatedSchemaUsage, | ||
| serializationFormats: (schema as SchemaUsage).serializationFormats?.filter( | ||
| (it) => it !== KnownMediaType.Multipart, | ||
| ), | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.