diff --git a/.chronus/changes/endpoint_design-2024-2-21-17-38-14.md b/.chronus/changes/endpoint_design-2024-2-21-17-38-14.md new file mode 100644 index 0000000000..3c90d16ffa --- /dev/null +++ b/.chronus/changes/endpoint_design-2024-2-21-17-38-14.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +create SdkEndpointType to encapsulate templating and url \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index 50c95b661f..5a1a507437 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -57,8 +57,6 @@ export interface SdkClientType { methods: SdkMethod[]; apiVersions: string[]; nameSpace: string; // fully qualified - endpoint: string; - hasParameterizedEndpoint: boolean; arm: boolean; } @@ -92,7 +90,8 @@ export type SdkType = | SdkConstantType | SdkUnionType | SdkModelType - | SdkCredentialType; + | SdkCredentialType + | SdkEndpointType; export interface SdkBuiltInType extends SdkTypeBase { kind: SdkBuiltInKinds; @@ -291,6 +290,12 @@ export interface SdkCredentialType extends SdkTypeBase { scheme: HttpAuth; } +export interface SdkEndpointType extends SdkTypeBase { + kind: "endpoint"; + serverUrl?: string; + templateArguments: SdkPathParameter[]; +} + export interface SdkModelPropertyTypeBase { __raw?: ModelProperty; type: SdkType; @@ -315,6 +320,7 @@ export interface SdkEndpointParameter extends SdkModelPropertyTypeBase { urlEncode: boolean; onClient: true; serializedName?: string; + type: SdkEndpointType; } export interface SdkCredentialParameter extends SdkModelPropertyTypeBase { diff --git a/packages/typespec-client-generator-core/src/lib.ts b/packages/typespec-client-generator-core/src/lib.ts index 8ada1f4a8c..42504efead 100644 --- a/packages/typespec-client-generator-core/src/lib.ts +++ b/packages/typespec-client-generator-core/src/lib.ts @@ -111,6 +111,12 @@ export const $lib = createTypeSpecLibrary({ default: paramMessage`Multiple services found in definition. Only one service is supported, so we will choose the first one ${"service"}`, }, }, + "server-param-not-path": { + severity: "error", + messages: { + default: paramMessage`Template argument ${"templateArgumentName"} is not a path parameter, it is a ${"templateArgumentType"}. It has to be a path.`, + }, + }, }, }); diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 977d2ec3a7..41a1644feb 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -8,7 +8,7 @@ import { getService, isErrorModel, } from "@typespec/compiler"; -import { HttpOperation, getHeaderFieldName, getServers, isContentTypeHeader } from "@typespec/http"; +import { HttpOperation, getHeaderFieldName, isContentTypeHeader } from "@typespec/http"; import { resolveVersions } from "@typespec/versioning"; import { getAccess, @@ -21,7 +21,6 @@ import { SdkClient, SdkClientType, SdkContext, - SdkEndpointParameter, SdkEnumType, SdkHeaderParameter, SdkHttpOperation, @@ -54,10 +53,8 @@ import { getClientNamespaceStringHelper, getDocHelper, getHashForType, - getSdkTypeBaseHelper, isAcceptHeader, isNullable, - updateWithApiVersionInformation, } from "./internal-utils.js"; import { getClientNamespaceString, @@ -72,6 +69,7 @@ import { getAllModelsWithDiagnostics, getClientTypeWithDiagnostics, getSdkCredentialParameter, + getSdkEndpointParameter, getSdkModelPropertyType, } from "./types.js"; @@ -600,116 +598,6 @@ function getSdkServiceMethod< return getSdkBasicServiceMethod(context, operation); } -function getDefaultSdkEndpointParameter< - TOptions extends object, - TServiceOperation extends SdkServiceOperation, ->( - context: SdkContext, - client: SdkClient, - serverUrl?: string -): SdkEndpointParameter[] { - let type: SdkType; - if (serverUrl) { - // this is a fixed endpoint so we model it as a constant type - type = { - ...getSdkTypeBaseHelper(context, client.service, "constant"), - value: serverUrl, - valueType: { - kind: "string", - encode: "string", - nullable: false, - }, - }; - } else { - // this is a parameterized endpoint - type = { - ...getSdkTypeBaseHelper(context, client.service, "string"), - encode: "string", - }; - } - const name = "endpoint"; - return [ - { - kind: "endpoint", - nameInClient: name, - name, - description: "Service host", - onClient: true, - urlEncode: false, - apiVersions: getAvailableApiVersions(context, client.type), - type: type, - optional: false, - isApiVersionParam: false, - nullable: false, - }, - ]; -} - -interface EndpointAndEndpointParameters { - endpoint: string; - properties: SdkEndpointParameter[]; - hasParameterizedEndpoint: boolean; -} - -function getEndpointAndEndpointParameters< - TOptions extends object, - TServiceOperation extends SdkServiceOperation, ->( - context: SdkContext, - client: SdkClient -): [EndpointAndEndpointParameters, readonly Diagnostic[]] { - const diagnostics = createDiagnosticCollector(); - const servers = getServers(context.program, client.service); - if (servers === undefined) { - return diagnostics.wrap({ - endpoint: "", - properties: getDefaultSdkEndpointParameter(context, client), - hasParameterizedEndpoint: false, - }); - } - if (servers.length > 1) { - return diagnostics.wrap({ - endpoint: "{endpoint}", - properties: getDefaultSdkEndpointParameter(context, client), - hasParameterizedEndpoint: true, - }); - } - if (servers[0].parameters.size === 0) { - return diagnostics.wrap({ - endpoint: servers[0].url, - properties: getDefaultSdkEndpointParameter( - context, - client, - servers[0].url - ), - hasParameterizedEndpoint: false, - }); - } - const properties: SdkEndpointParameter[] = []; - for (const param of servers[0].parameters.values()) { - const sdkParam = diagnostics.pipe( - getSdkModelPropertyType(context, param, { isEndpointParam: true }) - ); - if (sdkParam.kind !== "path") { - throw new Error("blah"); - } - properties.push({ - ...sdkParam, - kind: "endpoint", - urlEncode: false, - optional: false, - ...updateWithApiVersionInformation(context, param), - onClient: true, - description: sdkParam.description ?? servers[0].description, - }); - } - return diagnostics.wrap({ - endpoint: servers[0].url, - properties, - hasParameterizedEndpoint: true, - }); -} - function getClientDefaultApiVersion< TOptions extends object, TServiceOperation extends SdkServiceOperation, @@ -731,10 +619,9 @@ function getSdkInitializationType< ): [SdkInitializationType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const credentialParam = getSdkCredentialParameter(context, client); - const endpointInfo = diagnostics.pipe( - getEndpointAndEndpointParameters(context, client) - ); - const properties: SdkParameter[] = endpointInfo.properties; + const properties: SdkParameter[] = [ + diagnostics.pipe(getSdkEndpointParameter(context, client)), // there will always be an endpoint parameter + ]; if (credentialParam) { properties.push(credentialParam); } @@ -809,10 +696,6 @@ function createSdkClientType< // NOTE: getSdkMethods recursively calls createSdkClientType const methods = diagnostics.pipe(getSdkMethods(context, client, baseClientType)); - - const endpointInfo = diagnostics.pipe( - getEndpointAndEndpointParameters(context, client) - ); const docWrapper = getDocHelper(context, baseClientType.type); const sdkClientType: SdkClientType = { kind: "client", @@ -825,8 +708,6 @@ function createSdkClientType< initialization: isClient ? diagnostics.pipe(getSdkInitializationType(context, client)) // MUST call this after getSdkMethods has been called : undefined, - endpoint: endpointInfo.endpoint, - hasParameterizedEndpoint: endpointInfo.hasParameterizedEndpoint, arm: client.arm, }; context.__clients!.push(sdkClientType); diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 9c43a43197..2b4e79de10 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -70,10 +70,13 @@ import { SdkDatetimeType, SdkDictionaryType, SdkDurationType, + SdkEndpointParameter, + SdkEndpointType, SdkEnumType, SdkEnumValueType, SdkModelPropertyType, SdkModelType, + SdkPathParameter, SdkTupleType, SdkType, SdkUnionType, @@ -996,10 +999,18 @@ export function getSdkModelPropertyType( optional: type.optional, }); } else if (isPathParam(program, type) || options.isEndpointParam) { + // we don't url encode if the type can be assigned to url + const urlEncode = !ignoreDiagnostics( + program.checker.isTypeAssignableTo( + type.type.projectionBase ?? type.type, + program.checker.getStdType("url"), + type.type + ) + ); return diagnostics.wrap({ ...base, kind: "path", - urlEncode: true, + urlEncode, serializedName: getPathParamName(program, type), ...updateWithApiVersionInformation(context, type), optional: false, @@ -1062,6 +1073,69 @@ export function getSdkModelPropertyType( } } +export function getSdkEndpointParameter( + context: TCGCContext, + client: SdkClient +): [SdkEndpointParameter, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const servers = getServers(context.program, client.service); + let type: SdkEndpointType; + let optional: boolean = false; + if (servers === undefined || servers.length > 1) { + // if there is no defined server url, or if there are more than one + // we will return a mandatory endpoint parameter in initialization + type = { + kind: "endpoint", + nullable: false, + templateArguments: [], + }; + } else { + // this means we have one server + const templateArguments: SdkPathParameter[] = []; + type = { + kind: "endpoint", + nullable: false, + serverUrl: servers[0].url, + templateArguments, + }; + for (const param of servers[0].parameters.values()) { + const sdkParam = diagnostics.pipe( + getSdkModelPropertyType(context, param, { isEndpointParam: true }) + ); + if (sdkParam.kind === "path") { + templateArguments.push(sdkParam); + sdkParam.description = sdkParam.description ?? servers[0].description; + sdkParam.onClient = true; + } else { + diagnostics.add( + createDiagnostic({ + code: "server-param-not-path", + target: param, + format: { + templateArgumentName: sdkParam.name, + templateArgumentType: sdkParam.kind, + }, + }) + ); + } + } + optional = !!servers[0].url.length && templateArguments.every((param) => param.optional); + } + return diagnostics.wrap({ + kind: "endpoint", + type, + nameInClient: "endpoint", + name: "endpoint", + description: "Service host", + onClient: true, + urlEncode: false, + apiVersions: getAvailableApiVersions(context, client.service), + optional, + isApiVersionParam: false, + nullable: false, + }); +} + function addPropertiesToModelType( context: TCGCContext, type: Model, diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index a5893718b6..5c9897a253 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -102,10 +102,11 @@ describe("typespec-client-generator-core: package", () => { strictEqual(endpointParam.nameInClient, "endpoint"); strictEqual(endpointParam.name, "endpoint"); strictEqual(endpointParam.onClient, true); - strictEqual(endpointParam.optional, false); - strictEqual(endpointParam.type.kind, "constant"); - strictEqual(endpointParam.type.value, "http://localhost:3000"); + strictEqual(endpointParam.optional, true); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); strictEqual(endpointParam.urlEncode, false); + strictEqual(endpointParam.type.templateArguments.length, 0); }); it("initialization default endpoint with apikey auth", async () => { @@ -119,7 +120,6 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const client = sdkPackage.clients[0]; strictEqual(client.name, "ServiceClient"); - strictEqual(client.endpoint, "http://localhost:3000"); const initialization = client.initialization; ok(initialization); strictEqual(initialization.properties.length, 2); @@ -127,8 +127,9 @@ describe("typespec-client-generator-core: package", () => { const endpointParam = initialization.properties.filter( (p): p is SdkEndpointParameter => p.kind === "endpoint" )[0]; - strictEqual(endpointParam.type.kind, "constant"); - strictEqual(endpointParam.type.value, "http://localhost:3000"); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); + strictEqual(endpointParam.type.templateArguments.length, 0); const credentialParam = initialization.properties.filter( (p): p is SdkCredentialParameter => p.kind === "credential" @@ -169,8 +170,9 @@ describe("typespec-client-generator-core: package", () => { const endpointParam = initialization.properties.filter( (p): p is SdkEndpointParameter => p.kind === "endpoint" )[0]; - strictEqual(endpointParam.type.kind, "constant"); - strictEqual(endpointParam.type.value, "http://localhost:3000"); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); + strictEqual(endpointParam.type.templateArguments.length, 0); const credentialParam = initialization.properties.filter( (p): p is SdkCredentialParameter => p.kind === "credential" @@ -217,8 +219,8 @@ describe("typespec-client-generator-core: package", () => { const endpointParam = initialization.properties.filter( (p): p is SdkEndpointParameter => p.kind === "endpoint" )[0]; - strictEqual(endpointParam.type.kind, "constant"); - strictEqual(endpointParam.type.value, "http://localhost:3000"); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); const credentialParam = initialization.properties.filter( (p): p is SdkCredentialParameter => p.kind === "credential" @@ -271,7 +273,6 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const client = sdkPackage.clients[0]; strictEqual(client.name, "ServiceClient"); - strictEqual(client.endpoint, "{endpointInput}"); const initialization = client.initialization; ok(initialization); strictEqual(initialization.properties.length, 2); @@ -282,12 +283,21 @@ describe("typespec-client-generator-core: package", () => { strictEqual(endpointParam.clientDefaultValue, undefined); strictEqual(endpointParam.urlEncode, false); //eslint-disable-next-line deprecation/deprecation - strictEqual(endpointParam.nameInClient, "endpointInput"); - strictEqual(endpointParam.name, "endpointInput"); + strictEqual(endpointParam.nameInClient, "endpoint"); + strictEqual(endpointParam.name, "endpoint"); + strictEqual(endpointParam.type.kind, "endpoint"); strictEqual(endpointParam.onClient, true); strictEqual(endpointParam.optional, false); strictEqual(endpointParam.kind, "endpoint"); - strictEqual(endpointParam.description, "Testserver endpoint"); + strictEqual(endpointParam.type.templateArguments.length, 1); + const templateArg = endpointParam.type.templateArguments[0]; + strictEqual(templateArg.kind, "path"); + strictEqual(templateArg.name, "endpointInput"); + strictEqual(templateArg.urlEncode, false); + strictEqual(templateArg.optional, false); + strictEqual(templateArg.onClient, true); + strictEqual(templateArg.clientDefaultValue, undefined); + strictEqual(templateArg.description, "Testserver endpoint"); const credentialParam = initialization.properties.filter( (p): p is SdkCredentialParameter => p.kind === "credential" @@ -331,17 +341,16 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const client = sdkPackage.clients[0]; strictEqual(client.name, "ServiceClient"); - strictEqual(client.endpoint, "{endpoint}/server/path/multiple/{apiVersion}"); const initialization = client.initialization; ok(initialization); - strictEqual(initialization.properties.length, 3); + strictEqual(initialization.properties.length, 2); strictEqual(client.apiVersions.length, 1); strictEqual(client.apiVersions[0], "v1.0"); const endpointParams = initialization.properties.filter( (p): p is SdkEndpointParameter => p.kind === "endpoint" ); - strictEqual(endpointParams.length, 2); + strictEqual(endpointParams.length, 1); const endpointParam = endpointParams[0]; strictEqual(endpointParam.clientDefaultValue, undefined); strictEqual(endpointParam.urlEncode, false); @@ -352,20 +361,32 @@ describe("typespec-client-generator-core: package", () => { strictEqual(endpointParam.optional, false); strictEqual(endpointParam.kind, "endpoint"); - const apiVersionParam = endpointParams[1]; + const endpointParamType = endpointParam.type; + strictEqual(endpointParamType.kind, "endpoint"); + strictEqual(endpointParamType.serverUrl, "{endpoint}/server/path/multiple/{apiVersion}"); + + strictEqual(endpointParamType.templateArguments.length, 2); + const endpointTemplateArg = endpointParamType.templateArguments[0]; + strictEqual(endpointTemplateArg.name, "endpoint"); + strictEqual(endpointTemplateArg.onClient, true); + strictEqual(endpointTemplateArg.optional, false); + strictEqual(endpointTemplateArg.kind, "path"); + + const apiVersionParam = endpointParamType.templateArguments[1]; strictEqual(apiVersionParam.clientDefaultValue, "v1.0"); - strictEqual(apiVersionParam.urlEncode, false); + strictEqual(apiVersionParam.urlEncode, true); //eslint-disable-next-line deprecation/deprecation strictEqual(apiVersionParam.nameInClient, "apiVersion"); strictEqual(apiVersionParam.name, "apiVersion"); strictEqual(apiVersionParam.onClient, true); strictEqual(apiVersionParam.optional, false); - strictEqual(apiVersionParam.kind, "endpoint"); + strictEqual(apiVersionParam.kind, "path"); deepStrictEqual(client.apiVersions, ["v1.0"]); - const credentialParam = initialization.properties.filter( + const credentialParam = initialization.properties.find( (p): p is SdkCredentialParameter => p.kind === "credential" - )[0]; + ); + ok(credentialParam); //eslint-disable-next-line deprecation/deprecation strictEqual(credentialParam.nameInClient, "credential"); strictEqual(credentialParam.name, "credential"); @@ -420,13 +441,21 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const client = sdkPackage.clients[0]; strictEqual(client.name, "ServiceClient"); - strictEqual(client.endpoint, "http://localhost:3000"); const initialization = client.initialization; ok(initialization); strictEqual(initialization.properties.length, 3); strictEqual(client.apiVersions.length, 1); strictEqual(client.apiVersions[0], "2022-12-01-preview"); + const endpointParam = initialization.properties.find((x) => x.kind === "endpoint"); + ok(endpointParam); + strictEqual(endpointParam.name, "endpoint"); + strictEqual(endpointParam.kind, "endpoint"); + strictEqual(endpointParam.optional, true); + strictEqual(endpointParam.onClient, true); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); + const apiVersionParam = initialization.properties.filter((p) => p.isApiVersionParam)[0]; //eslint-disable-next-line deprecation/deprecation strictEqual(apiVersionParam.nameInClient, "apiVersion"); @@ -483,13 +512,17 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const client = sdkPackage.clients[0]; strictEqual(client.name, "ServiceClient"); - strictEqual(client.endpoint, "http://localhost:3000"); const initialization = client.initialization; ok(initialization); strictEqual(initialization.properties.length, 3); strictEqual(client.apiVersions.length, 2); deepStrictEqual(client.apiVersions, ["2022-12-01-preview", "2022-12-01"]); + const endpointParam = initialization.properties.find((x) => x.kind === "endpoint"); + ok(endpointParam); + strictEqual(endpointParam.type.kind, "endpoint"); + strictEqual(endpointParam.type.serverUrl, "http://localhost:3000"); + const apiVersionParam = initialization.properties.filter((p) => p.isApiVersionParam)[0]; //eslint-disable-next-line deprecation/deprecation strictEqual(apiVersionParam.nameInClient, "apiVersion");