diff --git a/.chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md b/.chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md new file mode 100644 index 0000000000..01b4ee5937 --- /dev/null +++ b/.chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md @@ -0,0 +1,9 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +1. Introduce new usage: `LroInitial`, `LroPolling`, `LroFinalEnvelope`. +2. usage and access now properly propagate on polling model, final result and final envelop result of `lroMetadata`. diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index f1ee226d54..c53f636a39 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -751,6 +751,12 @@ export enum UsageFlags { Xml = 1 << 9, // Set when type is used for exception output. Exception = 1 << 10, + // Set when type is used as LRO initial response. + LroInitial = 1 << 11, + // Set when type is used as LRO polling response. + LroPolling = 1 << 12, + // Set when type is used as LRO final envelop response. + LroFinalEnvelope = 1 << 13, } interface SdkExampleBase { diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 923cb76a53..43625d4842 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1596,6 +1596,7 @@ function updateTypesFromOperation( diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); } + const lroMetaData = getLroMetadata(program, operation); for (const response of httpOperation.responses) { for (const innerResponse of response.responses) { if (innerResponse.body?.type && !isNeverOrVoidType(innerResponse.body.type)) { @@ -1607,6 +1608,9 @@ function updateTypesFromOperation( if (generateConvenient) { if (response.statusCodes === "*" || isErrorModel(context.program, body)) { diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.Exception, sdkType)); + } else if (lroMetaData !== undefined) { + // when the operation is an lro, the response should be its initial response. + diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.LroInitial, sdkType)); } else { diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.Output, sdkType)); } @@ -1634,29 +1638,35 @@ function updateTypesFromOperation( } } } - const lroMetaData = getLroMetadata(program, operation); + if (lroMetaData && generateConvenient) { - if (lroMetaData.finalResult !== undefined && lroMetaData.finalResult !== "void") { - const sdkType = diagnostics.pipe( - getClientTypeWithDiagnostics(context, lroMetaData.finalResult, operation), - ); - diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.Output, sdkType)); - const access = getAccessOverride(context, operation) ?? "public"; - diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); - - if (!context.arm) { - // TODO: currently skipping adding of envelopeResult due to arm error - // https://github.com/Azure/typespec-azure/issues/311 - const sdkType = diagnostics.pipe( - getClientTypeWithDiagnostics(context, lroMetaData.envelopeResult, operation), - ); - diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.Output, sdkType)); - const access = getAccessOverride(context, operation) ?? "public"; - diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); - } - } + // the final result will be normal output usage. + updateUsageOrAccessForLroComponent(lroMetaData.finalResult, UsageFlags.Output); + + // the final envelope result will have LroFinalEnvelope. + updateUsageOrAccessForLroComponent( + lroMetaData.finalEnvelopeResult, + UsageFlags.LroFinalEnvelope, + ); + + // the polling model will have LroPolling. + updateUsageOrAccessForLroComponent( + lroMetaData.pollingInfo.responseModel, + UsageFlags.LroPolling, + ); } return diagnostics.wrap(undefined); + + function updateUsageOrAccessForLroComponent( + model: Model | "void" | undefined, + usage: UsageFlags, + ) { + if (model === undefined || model === "void") return; + const sdkType = diagnostics.pipe(getClientTypeWithDiagnostics(context, model, operation)); + diagnostics.pipe(updateUsageOrAccess(context, usage, sdkType)); + const access = getAccessOverride(context, operation) ?? "public"; + diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); + } } function updateAccessOverride(context: TCGCContext): [void, readonly Diagnostic[]] { diff --git a/packages/typespec-client-generator-core/test/methods/lro.test.ts b/packages/typespec-client-generator-core/test/methods/lro.test.ts index b39abad310..af9bf3c002 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -4,8 +4,8 @@ import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-res import { OpenAPITestLibrary } from "@typespec/openapi/testing"; import { ok, strictEqual } from "assert"; import { assert, beforeEach, describe, it } from "vitest"; -import { SdkHttpOperation, SdkLroServiceMethod } from "../../src/interfaces.js"; -import { createSdkTestRunner, SdkTestRunner } from "../test-host.js"; +import { UsageFlags } from "../../src/interfaces.js"; +import { createSdkTestRunner, hasFlag, SdkTestRunner } from "../test-host.js"; describe("typespec-client-generator-core: long running operation metadata", () => { let runner: SdkTestRunner; @@ -69,8 +69,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = method.parameters.map((m) => m.type), roundtripModel, ); + const initialResponse = method.response.type; + ok(initialResponse); + strictEqual(initialResponse.kind, "model"); + assert.isTrue( + hasFlag(initialResponse.usage, UsageFlags.LroInitial), + "the response of a lro method should have the usage of LroInitial", + ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.originalUri); assert.isUndefined(metadata.finalStep); @@ -79,10 +86,26 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); strictEqual(metadata.finalResponse?.envelopeResult, roundtripModel); strictEqual(metadata.finalResponse?.result, roundtripModel); + assert.isTrue( + hasFlag(roundtripModel.usage, UsageFlags.LroFinalEnvelope), + "roundtrip model should have the usage of LroFinalEnvelope", + ); assert.isUndefined(metadata.finalResponse?.resultPath); }); @@ -105,13 +128,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = const method = methods[0]; strictEqual(method.kind, "lro"); strictEqual(method.name, "delete"); - const lroMethod = method as SdkLroServiceMethod; assert.notInclude( - lroMethod.operation.parameters.map((m) => m.kind), + method.operation.parameters.map((m) => m.kind), "body", ); - const metadata = lroMethod.lroMetadata; + const initialResponse = method.response.type; + strictEqual(initialResponse, undefined); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "noPollingResult"); @@ -120,6 +145,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); @@ -158,7 +195,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + ok(initialResponse); + strictEqual(initialResponse.kind, "model"); + assert.isTrue( + hasFlag(initialResponse.usage, UsageFlags.LroInitial), + "the response of a lro method should have the usage of LroInitial", + ); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty"); @@ -167,6 +212,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusExportedUserError", ); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); const returnModel = runner.context.sdkPackage.models.find((m) => m.name === "ExportedUser"); @@ -175,6 +232,10 @@ describe("typespec-client-generator-core: long running operation metadata", () = strictEqual(metadata.finalResponse?.envelopeResult, pollingModel); strictEqual(metadata.finalResponse?.result, returnModel); strictEqual(metadata.finalResponse?.resultPath, "result"); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroFinalEnvelope), + "the polling model here is also the final envelope model, it should have the usage of LroFinalEnvelope", + ); }); }); @@ -224,7 +285,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = inputModel, ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + ok(initialResponse); + strictEqual(initialResponse.kind, "model"); + assert.isTrue( + hasFlag(initialResponse.usage, UsageFlags.LroInitial), + "the response of a lro method should have the usage of LroInitial", + ); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty"); @@ -233,6 +302,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusGenerationResultError", ); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); const returnModel = runner.context.sdkPackage.models.find( @@ -242,6 +323,10 @@ describe("typespec-client-generator-core: long running operation metadata", () = strictEqual(metadata.finalResponse?.envelopeResult, pollingModel); strictEqual(metadata.finalResponse?.result, returnModel); strictEqual(metadata.finalResponse?.resultPath, "result"); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroFinalEnvelope), + "the polling model here is also the final envelope model, it should have the usage of LroFinalEnvelope", + ); }); }); describe("Custom LRO", () => { @@ -294,13 +379,28 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "noPollingResult"); const pollingModel = runner.context.sdkPackage.models.find((m) => m.name === "JobState"); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.Output), + "this polling model has output usage because there is a polling operation returning it", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); @@ -359,18 +459,93 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.location); strictEqual(metadata.finalStep?.kind, "noPollingResult"); const pollingModel = runner.context.sdkPackage.models.find((m) => m.name === "JobState"); ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.Output), + "this polling model has output usage because there is a polling operation returning it", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); }); }); + + it("LRO defined in different namespace", async () => { + await runner.compile(` + @service({}) + @versioned(Versions) + namespace TestClient { + enum Versions { + @useDependency(Azure.Core.Versions.v1_0_Preview_1) + v1: "v1", + @useDependency(Azure.Core.Versions.v1_0_Preview_2) + v2: "v2", + } + + alias ResourceOperations = global.Azure.Core.ResourceOperations; + + model PollResponse { + operationId: string; + status: Azure.Core.Foundations.OperationState; + } + + @pollingOperation(NonService.poll) + @post + @route("/post") + op longRunning(): AcceptedResponse; + } + + @useDependency(Azure.Core.Versions.v1_0_Preview_2, TestClient.Versions.v2) + namespace NonService { + @route("/poll") + @get + op poll(): TestClient.PollResponse; + }`); + const method = runner.context.sdkPackage.clients[0].methods.find( + (m) => m.name === "longRunning", + ); + ok(method); + strictEqual(method.kind, "lro"); + + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + + const metadata = method.lroMetadata; + ok(metadata); + const pollingModel = metadata.pollingStep.responseBody; + ok(pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); + }); }); describe("Arm LRO templates", () => { @@ -415,6 +590,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = const methods = runner.context.sdkPackage.clients[0].methods; strictEqual(methods.length, 1); const method = methods[0]; + strictEqual(method.kind, "lro"); strictEqual(method.name, "createOrReplace"); assert.include( method.parameters.map((m) => m.name), @@ -424,28 +600,46 @@ describe("typespec-client-generator-core: long running operation metadata", () = method.parameters.map((m) => m.name), "resource", ); + + const initialResponse = method.response.type; + ok(initialResponse); + strictEqual(initialResponse.kind, "model"); + assert.isTrue( + hasFlag(initialResponse.usage, UsageFlags.LroInitial), + "the response of a lro method should have the usage of LroInitial", + ); + const roundtripModel = runner.context.sdkPackage.models.find((m) => m.name === "Employee"); ok(roundtripModel); + strictEqual( + initialResponse, + roundtripModel, + "in this case the initial response is the same as the final", + ); assert.include( method.parameters.map((m) => m.type), roundtripModel, ); + // validate the model should be roundtrip here + assert.isTrue( + hasFlag(roundtripModel.usage, UsageFlags.Input | UsageFlags.Output), + "model should be input and output", + ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.azureAsyncOperation); strictEqual(metadata.finalStep?.kind, "finalOperationLink"); // ARM LRO core types are different - // const pollingModel = runner.context.sdkPackage.models.find( - // (m) => m.name === "ArmOperationStatusResourceProvisioningState" - // ); - // ok(pollingModel); - // strictEqual(metadata.pollingStep.responseBody, pollingModel); - // TODO: TCGC bug to not include polling model https://github.com/Azure/typespec-azure/issues/1530 - strictEqual( - metadata.pollingStep.responseBody?.name, - "ArmOperationStatusResourceProvisioningState", + const pollingModel = runner.context.sdkPackage.models.find( + (m) => m.name === "ArmOperationStatusResourceProvisioningState", + ); + ok(pollingModel); + strictEqual(metadata.pollingStep.responseBody, pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", ); strictEqual(metadata.finalResponse?.envelopeResult, roundtripModel); @@ -470,6 +664,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = const methods = runner.context.sdkPackage.clients[0].methods; strictEqual(methods.length, 1); const method = methods[0]; + strictEqual(method.kind, "lro"); strictEqual(method.name, "delete"); assert.include( method.parameters.map((m) => m.name), @@ -480,21 +675,31 @@ describe("typespec-client-generator-core: long running operation metadata", () = "resource", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.location); strictEqual(metadata.finalStep?.kind, "finalOperationLink"); // ARM LRO core types are different - // const pollingModel = runner.context.sdkPackage.models.find( - // (m) => m.name === "ArmOperationStatusResourceProvisioningState" - // ); - // ok(pollingModel); - // strictEqual(metadata.pollingStep.responseBody, pollingModel); - // TODO: TCGC bug to not include polling model - strictEqual( - metadata.pollingStep.responseBody?.name, - "ArmOperationStatusResourceProvisioningState", + const pollingModel = runner.context.sdkPackage.models.find( + (m) => m.name === "ArmOperationStatusResourceProvisioningState", + ); + ok(pollingModel); + strictEqual(metadata.pollingStep.responseBody, pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", ); assert.isUndefined(metadata.finalResponse); @@ -516,24 +721,39 @@ describe("typespec-client-generator-core: long running operation metadata", () = const methods = runner.context.sdkPackage.clients[0].methods; strictEqual(methods.length, 1); const method = methods[0]; + strictEqual(method.kind, "lro"); strictEqual(method.name, "actionAsync"); assert.include( method.parameters.map((m) => m.name), "employeeName", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.location); strictEqual(metadata.finalStep?.kind, "finalOperationLink"); // ARM LRO core types are different - // const pollingModel = runner.context.sdkPackage.models.find( - // (m) => m.name === "ArmOperationStatusResourceProvisioningState" - // ); - // ok(pollingModel); - // strictEqual(metadata.pollingStep.responseBody, pollingModel); - // TODO: TCGC bug to not include polling model + const pollingModel = runner.context.sdkPackage.models.find( + (m) => m.name === "ArmOperationStatusResourceProvisioningState", + ); + ok(pollingModel); + strictEqual(metadata.pollingStep.responseBody, pollingModel); + assert.isTrue( + hasFlag(pollingModel.usage, UsageFlags.LroPolling), + "polling model should have the usage of LroPolling", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Output), + "polling model should not be output", + ); + assert.isFalse( + hasFlag(pollingModel.usage, UsageFlags.Input), + "polling model should not be input", + ); strictEqual( metadata.pollingStep.responseBody?.name, "ArmOperationStatusResourceProvisioningState", diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index b8b6f62b01..e25a16d13b 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -1256,8 +1256,8 @@ describe("typespec-client-generator-core: package", () => { } `); const sdkPackage = runnerWithCore.context.sdkPackage; - strictEqual(sdkPackage.models.length, 0); - strictEqual(sdkPackage.enums.length, 1); + strictEqual(sdkPackage.models.length, 1); + strictEqual(sdkPackage.enums.length, 2); }); }); }); diff --git a/packages/typespec-client-generator-core/test/test-host.ts b/packages/typespec-client-generator-core/test/test-host.ts index 91a3a14dd3..1237ebc9c6 100644 --- a/packages/typespec-client-generator-core/test/test-host.ts +++ b/packages/typespec-client-generator-core/test/test-host.ts @@ -285,3 +285,7 @@ export function removeRawFromType(type: TType): TType { const { __raw, ...rest } = type as any; return rest; } + +export function hasFlag(value: T, flag: T): boolean { + return (value & flag) !== 0; +}