Skip to content

Commit

Permalink
Propogate api version (#805)
Browse files Browse the repository at this point in the history
fixes #802

---------

Co-authored-by: iscai-msft <[email protected]>
Co-authored-by: tadelesh <[email protected]>
  • Loading branch information
3 people authored May 9, 2024
1 parent 3706e67 commit a84c27d
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 36 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/propogate_api_version-2024-4-9-17-53-45.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

propagate api version from parent if not explicitly set
2 changes: 1 addition & 1 deletion packages/typespec-client-generator-core/src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ export function createSdkContext<
apiVersion: options?.versionStrategy === "ignore" ? "all" : context.options["api-version"],
originalProgram: context.program,
__namespaceToApiVersionParameter: new Map(),
__namespaceToApiVersions: new Map(),
__tspTypeToApiVersions: new Map(),
__namespaceToApiVersionClientDefaultValue: new Map(),
};
sdkContext.experimental_sdkPackage = getSdkPackage(sdkContext);
Expand Down
9 changes: 2 additions & 7 deletions packages/typespec-client-generator-core/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,7 @@ function getSdkHttpParameters(
contentTypes: [],
defaultContentType: "application/json", // actual content type info is added later
isApiVersionParam: false,
apiVersions: getAvailableApiVersions(
context,
tspBody.type,
httpOperation.operation.namespace
),
apiVersions: getAvailableApiVersions(context, tspBody.type, httpOperation.operation),
type,
optional: false,
nullable: isNullable(tspBody.type),
Expand Down Expand Up @@ -427,7 +423,6 @@ function getSdkHttpResponseAndExceptions(
: innerResponse.body.type;
}
}

const sdkResponse: SdkHttpResponse = {
__raw: response,
kind: "http",
Expand All @@ -440,7 +435,7 @@ function getSdkHttpResponseAndExceptions(
apiVersions: getAvailableApiVersions(
context,
httpOperation.operation,
httpOperation.operation.namespace
httpOperation.operation
),
nullable: body ? isNullable(body) : true,
};
Expand Down
37 changes: 27 additions & 10 deletions packages/typespec-client-generator-core/src/internal-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ export function filterApiVersionsWithDecorators(
return retval;
}

function sortAndRemoveDuplicates(a: string[], b: string[], apiVersions: string[]): string[] {
const union = Array.from(new Set([...a, ...b]));
return apiVersions.filter((item) => union.includes(item));
}

/**
*
* @param context
Expand All @@ -167,20 +172,32 @@ export function filterApiVersionsWithDecorators(
export function getAvailableApiVersions(
context: TCGCContext,
type: Type,
namespace?: Namespace | Interface
wrapper?: Type
): string[] {
let cachedApiVersions: string[] = [];
if (namespace) {
cachedApiVersions = context.__namespaceToApiVersions.get(namespace) || [];
let wrapperApiVersions: string[] = [];
if (wrapper) {
wrapperApiVersions = context.__tspTypeToApiVersions.get(wrapper) || [];
}

const apiVersions =
cachedApiVersions ||
const allApiVersions =
getVersions(context.program, type)[1]
?.getVersions()
.map((x) => x.value);
.map((x) => x.value) || [];

const apiVersions = wrapperApiVersions.length ? wrapperApiVersions : allApiVersions;
if (!apiVersions) return [];
return filterApiVersionsWithDecorators(context, type, apiVersions);
const explicitlyDecorated = filterApiVersionsWithDecorators(context, type, apiVersions);
if (explicitlyDecorated.length) {
context.__tspTypeToApiVersions.set(type, explicitlyDecorated);
return explicitlyDecorated;
}
// we take the union of all of the api versions that the type is available on
// if it's called multiple times with diff wrappers, we want to make sure we have
// all of the possible api versions listed
const existing = context.__tspTypeToApiVersions.get(type) || [];
const retval = sortAndRemoveDuplicates(wrapperApiVersions, existing, allApiVersions);
context.__tspTypeToApiVersions.set(type, retval);
return retval;
}

interface DocWrapper {
Expand Down Expand Up @@ -303,7 +320,7 @@ export interface TCGCContext {
httpOperationCache?: Map<Operation, HttpOperation>;
unionsMap?: Map<Union, SdkUnionType>;
__namespaceToApiVersionParameter: Map<Interface | Namespace, SdkParameter>;
__namespaceToApiVersions: Map<Interface | Namespace, string[]>;
__tspTypeToApiVersions: Map<Type, string[]>;
__namespaceToApiVersionClientDefaultValue: Map<Interface | Namespace, string | undefined>;
knownScalars?: Record<string, SdkBuiltInKinds>;
diagnostics: readonly Diagnostic[];
Expand All @@ -321,7 +338,7 @@ export function createTCGCContext(program: Program): TCGCContext {
diagnostics: [],
originalProgram: program,
__namespaceToApiVersionParameter: new Map(),
__namespaceToApiVersions: new Map(),
__tspTypeToApiVersions: new Map(),
__namespaceToApiVersionClientDefaultValue: new Map(),
};
}
Expand Down
29 changes: 18 additions & 11 deletions packages/typespec-client-generator-core/src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,13 @@ function getSdkBasicServiceMethod<
): [SdkServiceMethod<TServiceOperation>, readonly Diagnostic[]] {
const diagnostics = createDiagnosticCollector();
const methodParameters: SdkMethodParameter[] = [];

// we have to calculate apiVersions first, so that the information is put
// in __tspTypeToApiVersions before we call parameters since method wraps parameter
const apiVersions = getAvailableApiVersions(
context,
operation,
getLocationOfOperation(operation)
);
const httpOperation = getHttpOperationWithCache(context, operation);
const parameters = httpOperation.parameters;
// path/query/header parameters
Expand Down Expand Up @@ -283,7 +289,7 @@ function getSdkBasicServiceMethod<
details: getDocHelper(context, operation).details,
operation: serviceOperation,
response,
apiVersions: getAvailableApiVersions(context, operation, getLocationOfOperation(operation)),
apiVersions,
getParameterMapping: function getParameterMapping(
serviceParam: SdkServiceParameter
): SdkModelPropertyType[] {
Expand Down Expand Up @@ -378,7 +384,7 @@ function getSdkInitializationType<
usage: UsageFlags.Input,
nullable: false,
crossLanguageDefinitionId: `${getNamespaceFullName(client.service.namespace!)}.${name}`,
apiVersions: getAvailableApiVersions(context, client.service, client.type),
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
isFormDataType: false,
isError: false,
});
Expand All @@ -393,12 +399,14 @@ function getSdkMethodParameter(
if (type.kind !== "ModelProperty") {
const libraryName = getLibraryName(context, type);
const name = camelCase(libraryName ?? "body");
// call before creating property type, so we can pass apiVersions of param onto its type
const apiVersions = getAvailableApiVersions(context, type, operation);
const propertyType = diagnostics.pipe(getClientTypeWithDiagnostics(context, type, operation));
return diagnostics.wrap({
kind: "method",
description: getDocHelper(context, type).description,
details: getDocHelper(context, type).details,
apiVersions: getAvailableApiVersions(context, type, getLocationOfOperation(operation)),
apiVersions,
type: propertyType,
nameInClient: name,
name,
Expand Down Expand Up @@ -481,7 +489,7 @@ function getSdkEndpointParameter(
encode: "string",
},
isApiVersionParam: false,
apiVersions: getAvailableApiVersions(context, client.service, client.type),
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
},
],
};
Expand All @@ -500,10 +508,10 @@ function getSdkEndpointParameter(
templateArguments.push(sdkParam);
sdkParam.description = sdkParam.description ?? servers[0].description;
sdkParam.onClient = true;
sdkParam.apiVersions = context.__namespaceToApiVersions.get(client.type) || [];
const apiVersionInfo = updateWithApiVersionInformation(context, param, client.type);
sdkParam.clientDefaultValue = apiVersionInfo.clientDefaultValue;
sdkParam.isApiVersionParam = apiVersionInfo.isApiVersionParam;
sdkParam.apiVersions = getAvailableApiVersions(context, param, client.type);
} else {
diagnostics.add(
createDiagnostic({
Expand All @@ -528,7 +536,7 @@ function getSdkEndpointParameter(
description: "Service host",
onClient: true,
urlEncode: false,
apiVersions: getAvailableApiVersions(context, client.service, client.type),
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
optional,
isApiVersionParam: false,
nullable: false,
Expand All @@ -545,7 +553,6 @@ function createSdkClientType<
const diagnostics = createDiagnosticCollector();
const isClient = client.kind === "SdkClient";
const clientName = isClient ? client.name : client.type.name;

// NOTE: getSdkMethods recursively calls createSdkClientType
const methods = diagnostics.pipe(getSdkMethods(context, client));
const docWrapper = getDocHelper(context, client.type);
Expand All @@ -555,7 +562,7 @@ function createSdkClientType<
description: docWrapper.description,
details: docWrapper.details,
methods: methods,
apiVersions: getAvailableApiVersions(context, client.type, client.type),
apiVersions: context.__tspTypeToApiVersions.get(client.type)!,
nameSpace: getClientNamespaceStringHelper(context, client.service)!,
initialization: diagnostics.pipe(
getSdkInitializationType<TOptions, TServiceOperation>(context, client)
Expand All @@ -572,7 +579,7 @@ function populateApiVersionInformation(context: SdkContext): void {
let clientApiVersions = resolveVersions(context.program, client.service)
.filter((x) => x.rootVersion)
.map((x) => x.rootVersion!.value);
context.__namespaceToApiVersions.set(
context.__tspTypeToApiVersions.set(
client.type,
filterApiVersionsWithDecorators(context, client.type, clientApiVersions)
);
Expand All @@ -585,7 +592,7 @@ function populateApiVersionInformation(context: SdkContext): void {
clientApiVersions = resolveVersions(context.program, og.service)
.filter((x) => x.rootVersion)
.map((x) => x.rootVersion!.value);
context.__namespaceToApiVersions.set(
context.__tspTypeToApiVersions.set(
og.type,
filterApiVersionsWithDecorators(context, og.type, clientApiVersions)
);
Expand Down
12 changes: 5 additions & 7 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@ function addDiscriminatorToModelType(
isGeneratedName: false,
onClient: false,
apiVersions: discriminatorProperty
? getAvailableApiVersions(context, discriminatorProperty.__raw!, type.namespace)
: getAvailableApiVersions(context, type, type.namespace),
? getAvailableApiVersions(context, discriminatorProperty.__raw!, type)
: getAvailableApiVersions(context, type, type),
isApiVersionParam: false,
isMultipartFileInput: false, // discriminator property cannot be a file
flatten: false, // discriminator properties can not be flattened
Expand Down Expand Up @@ -967,6 +967,8 @@ export function getSdkModelPropertyTypeBase(
operation?: Operation
): [SdkModelPropertyTypeBase, readonly Diagnostic[]] {
const diagnostics = createDiagnosticCollector();
// get api version info so we can cache info about its api versions before we get to property type level
const apiVersions = getAvailableApiVersions(context, type, operation || type.model);
let propertyType = diagnostics.pipe(getClientTypeWithDiagnostics(context, type.type, operation));
diagnostics.pipe(addEncodeInfo(context, type, propertyType));
addFormatInfo(context, type, propertyType);
Expand All @@ -980,11 +982,7 @@ export function getSdkModelPropertyTypeBase(
__raw: type,
description: docWrapper.description,
details: docWrapper.details,
apiVersions: getAvailableApiVersions(
context,
type,
operation?.interface || operation?.namespace || type.model?.namespace
),
apiVersions,
type: propertyType,
nameInClient: name,
name,
Expand Down
49 changes: 49 additions & 0 deletions packages/typespec-client-generator-core/test/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3326,6 +3326,55 @@ describe("typespec-client-generator-core: package", () => {
const sdkPackage = runner.context.experimental_sdkPackage;
strictEqual(sdkPackage.clients[0].methods[0].parameters[0].clientDefaultValue, "v2");
});
it("add method", async () => {
await runner.compileWithVersionedService(`
@route("/v1")
@post
@added(Versions.v2)
op v2(@header headerV2: string): void;
`);

const sdkPackage = runner.context.experimental_sdkPackage;
deepStrictEqual(sdkPackage.clients[0].apiVersions, ["v1", "v2"]);
const method = getServiceMethodOfClient(sdkPackage);
strictEqual(method.kind, "basic");
deepStrictEqual(method.apiVersions, ["v2"]);
strictEqual(method.parameters.length, 1);
const methodParam = sdkPackage.clients[0].methods[0].parameters[0];
strictEqual(methodParam.name, "headerV2");
strictEqual(methodParam.kind, "method");
deepStrictEqual(methodParam.apiVersions, ["v2"]);

strictEqual(method.operation.parameters.length, 1);
const headerParam = method.operation.parameters[0];
strictEqual(headerParam.name, "headerV2");
strictEqual(headerParam.kind, "header");
deepStrictEqual(headerParam.apiVersions, ["v2"]);
});
it("add parameter", async () => {
await runner.compileWithVersionedService(`
@route("/v1")
@post
op v1(@added(Versions.v2) @header headerV2: string): void;
`);

const sdkPackage = runner.context.experimental_sdkPackage;
deepStrictEqual(sdkPackage.clients[0].apiVersions, ["v1", "v2"]);
const method = getServiceMethodOfClient(sdkPackage);
strictEqual(method.kind, "basic");
deepStrictEqual(method.apiVersions, ["v1", "v2"]);
strictEqual(method.parameters.length, 1);
const methodParam = sdkPackage.clients[0].methods[0].parameters[0];
strictEqual(methodParam.name, "headerV2");
strictEqual(methodParam.kind, "method");
deepStrictEqual(methodParam.apiVersions, ["v2"]);

strictEqual(method.operation.parameters.length, 1);
const headerParam = method.operation.parameters[0];
strictEqual(headerParam.name, "headerV2");
strictEqual(headerParam.kind, "header");
deepStrictEqual(headerParam.apiVersions, ["v2"]);
});
});
});

Expand Down
34 changes: 34 additions & 0 deletions packages/typespec-client-generator-core/test/test-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface SdkTestRunner extends BasicTestRunner {
compileWithBuiltInService(code: string): Promise<Record<string, Type>>;
compileWithBuiltInAzureCoreService(code: string): Promise<Record<string, Type>>;
compileWithCustomization(mainCode: string, clientCode: string): Promise<Record<string, Type>>;
compileWithVersionedService(code: string): Promise<Record<string, Type>>;
compileAndDiagnoseWithCustomization(
mainCode: string,
clientCode: string
Expand Down Expand Up @@ -185,6 +186,39 @@ export async function createSdkTestRunner(
return result;
};

// compile with versioned service
sdkTestRunner.compileWithVersionedService = async function (code) {
const result = await baseCompile(
`
@service
@versioned(Versions)
@server(
"{endpoint}/versioning/api-version:{version}",
"Testserver endpoint",
{
endpoint: url,
version: Versions,
}
)
namespace Versioning;
enum Versions {
v1: "v1",
v2: "v2",
}
${code}`,
{
noEmit: true,
}
);
sdkTestRunner.context = createSdkContextTestHelper(
sdkTestRunner.program,
options,
options.emitterName
);
return result;
};

// compile and diagnose with client.tsp
sdkTestRunner.compileAndDiagnoseWithCustomization = async function (mainCode, clientCode) {
host.addTypeSpecFile("./main.tsp", `${mainAutoCode}${mainCode}`);
Expand Down

0 comments on commit a84c27d

Please sign in to comment.