From d05e51f1d34e97e8bb2143adb66774891aca05aa Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Tue, 19 Nov 2024 17:01:08 +0800 Subject: [PATCH 01/17] remove the workaround since #311 is resolved --- packages/typespec-client-generator-core/src/types.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 9d3ae45afb..f819d1be63 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1235,8 +1235,8 @@ function updateMultiPartInfo( : undefined, contentType: httpOperationPart.body.contentTypeProperty ? diagnostics.pipe( - getSdkModelPropertyType(context, httpOperationPart.body.contentTypeProperty, operation), - ) + getSdkModelPropertyType(context, httpOperationPart.body.contentTypeProperty, operation), + ) : undefined, defaultContentTypes: httpOperationPart.body.contentTypes, }; @@ -1643,9 +1643,7 @@ function updateTypesFromOperation( 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 + if (lroMetaData.envelopeResult !== undefined) { const sdkType = diagnostics.pipe( getClientTypeWithDiagnostics(context, lroMetaData.envelopeResult, operation), ); From c0d712ea8dc2cd1e81c547bd24befc8e0942968d Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 13:09:57 +0800 Subject: [PATCH 02/17] Create remove-lro-workaround-2024-10-19-9-4-1.md --- .../changes/remove-lro-workaround-2024-10-19-9-4-1.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md 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..5a3ed5bb1a --- /dev/null +++ b/.chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +remove the arm lro workaround From 8d8fd8c8f0faf3b765e0e1dbf18c235b1082a74d Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 20 Nov 2024 13:29:52 +0800 Subject: [PATCH 03/17] format --- packages/typespec-client-generator-core/src/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index f819d1be63..823ff61a89 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1235,8 +1235,8 @@ function updateMultiPartInfo( : undefined, contentType: httpOperationPart.body.contentTypeProperty ? diagnostics.pipe( - getSdkModelPropertyType(context, httpOperationPart.body.contentTypeProperty, operation), - ) + getSdkModelPropertyType(context, httpOperationPart.body.contentTypeProperty, operation), + ) : undefined, defaultContentTypes: httpOperationPart.body.contentTypes, }; From 83cf322741292148e4a0719e46886dd310445a98 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 15:23:15 +0800 Subject: [PATCH 04/17] add some test cases --- .../src/types.ts | 26 +++++++------------ .../test/methods/lro.test.ts | 11 +++++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 823ff61a89..14f1480f66 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1635,25 +1635,19 @@ 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)); + updateUsageOrAccessForLroComponent(lroMetaData.finalResult); - if (lroMetaData.envelopeResult !== undefined) { - 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)); - } - } + updateUsageOrAccessForLroComponent(lroMetaData.finalEnvelopeResult); } return diagnostics.wrap(undefined); + + function updateUsageOrAccessForLroComponent(model: Model | "void" | undefined) { + if (model === undefined || model === "void") return; + const sdkType = diagnostics.pipe(getClientTypeWithDiagnostics(context, model, operation)); + diagnostics.pipe(updateUsageOrAccess(context, UsageFlags.Output, 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..072f6eb016 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -4,7 +4,7 @@ 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 { SdkHttpOperation, SdkLroServiceMethod, UsageFlags } from "../../src/interfaces.js"; import { createSdkTestRunner, SdkTestRunner } from "../test-host.js"; describe("typespec-client-generator-core: long running operation metadata", () => { @@ -430,8 +430,12 @@ describe("typespec-client-generator-core: long running operation metadata", () = method.parameters.map((m) => m.type), roundtripModel, ); + // validate the model should be roundtrip here + assert.isDefined(roundtripModel.usage & UsageFlags.Input); + assert.isDefined(roundtripModel.usage & UsageFlags.Output); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + strictEqual(method.kind, "lro"); + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.azureAsyncOperation); strictEqual(metadata.finalStep?.kind, "finalOperationLink"); @@ -480,7 +484,8 @@ describe("typespec-client-generator-core: long running operation metadata", () = "resource", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + strictEqual(method.kind, "lro"); + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.location); strictEqual(metadata.finalStep?.kind, "finalOperationLink"); From 3edd23bba2f4a4166e95282497e3569c1bdaa6cc Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 15:38:40 +0800 Subject: [PATCH 05/17] update test cases --- .../src/types.ts | 2 ++ .../test/methods/lro.test.ts | 27 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 14f1480f66..211f41e721 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1638,6 +1638,8 @@ function updateTypesFromOperation( updateUsageOrAccessForLroComponent(lroMetaData.finalResult); updateUsageOrAccessForLroComponent(lroMetaData.finalEnvelopeResult); + + updateUsageOrAccessForLroComponent(lroMetaData.pollingInfo.responseModel); } return diagnostics.wrap(undefined); 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 072f6eb016..b717fbfd1e 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -2,7 +2,7 @@ import { FinalStateValue } from "@azure-tools/typespec-azure-core"; import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing"; import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-resource-manager/testing"; import { OpenAPITestLibrary } from "@typespec/openapi/testing"; -import { ok, strictEqual } from "assert"; +import { notStrictEqual, ok, strictEqual } from "assert"; import { assert, beforeEach, describe, it } from "vitest"; import { SdkHttpOperation, SdkLroServiceMethod, UsageFlags } from "../../src/interfaces.js"; import { createSdkTestRunner, SdkTestRunner } from "../test-host.js"; @@ -70,7 +70,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = roundtripModel, ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.originalUri); assert.isUndefined(metadata.finalStep); @@ -79,6 +79,8 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); strictEqual(metadata.finalResponse?.envelopeResult, roundtripModel); @@ -120,6 +122,8 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); @@ -158,7 +162,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty"); @@ -167,6 +171,8 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusExportedUserError", ); ok(pollingModel); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); const returnModel = runner.context.sdkPackage.models.find((m) => m.name === "ExportedUser"); @@ -224,7 +230,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = inputModel, ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation); strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty"); @@ -233,6 +239,8 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusGenerationResultError", ); ok(pollingModel); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); const returnModel = runner.context.sdkPackage.models.find( @@ -294,13 +302,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + 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); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); @@ -359,13 +369,15 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); - const metadata = (method as SdkLroServiceMethod).lroMetadata; + 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); + strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input + notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output strictEqual(metadata.pollingStep.responseBody, pollingModel); assert.isUndefined(metadata.finalResponse); @@ -431,8 +443,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = roundtripModel, ); // validate the model should be roundtrip here - assert.isDefined(roundtripModel.usage & UsageFlags.Input); - assert.isDefined(roundtripModel.usage & UsageFlags.Output); + notStrictEqual(roundtripModel.usage & (UsageFlags.Input | UsageFlags.Output), 0); strictEqual(method.kind, "lro"); const metadata = method.lroMetadata; From de2c7bafd579ea01d35311774c58b7fdf2d6f5ef Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 15:40:00 +0800 Subject: [PATCH 06/17] update changelog --- .chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 5a3ed5bb1a..bc182afc8b 100644 --- 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 @@ -5,4 +5,4 @@ packages: - "@azure-tools/typespec-client-generator-core" --- -remove the arm lro workaround +usage and access now properly propagate on polling model, final envelop result of `lroMetadata` From 9f98fcf0d69f32d6fd01f37f0393ad14ab56f6f9 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 16:16:47 +0800 Subject: [PATCH 07/17] update test cases --- packages/typespec-client-generator-core/test/package.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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); }); }); }); From fbe44b2cc6f750f5f98d5f52d904f2c6a7a007a1 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 16:38:15 +0800 Subject: [PATCH 08/17] add a new test case --- .../test/methods/lro.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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 b717fbfd1e..90513d4c22 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -383,6 +383,34 @@ describe("typespec-client-generator-core: long running operation metadata", () = assert.isUndefined(metadata.finalResponse); }); }); + + it("LRO defined in different namespace", async () => { + await runner.compileWithVersionedService(` + model PollResponse { + operationId: string; + status: Azure.Core.Foundations.OperationState; + } + + @pollingOperation(NonService.poll) + @post + @route("/post") + op longRunning(): AcceptedResponse; + + namespace NonService { + @route("/poll") + @get + op poll(): TestClient.PollResponse; + }`); + const methods = runner.context.sdkPackage.clients[0].methods; + + const method = methods.find((m) => m.name === "longRunning"); + ok(method); + strictEqual(method.kind, "lro"); + const metadata = method.lroMetadata; + ok(metadata); + ok(method.lroMetadata.pollingStep.responseBody); + notStrictEqual(method.lroMetadata.pollingStep.responseBody.usage & UsageFlags.Output, 0); // the polling model should be output + }); }); describe("Arm LRO templates", () => { From a19783c84528c6cb537b8f8f998d39102558b0a9 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 16:39:15 +0800 Subject: [PATCH 09/17] format --- .../typespec-client-generator-core/test/methods/lro.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 90513d4c22..a97c5dbc52 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -397,9 +397,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = op longRunning(): AcceptedResponse; namespace NonService { - @route("/poll") - @get - op poll(): TestClient.PollResponse; + @route("/poll") + @get + op poll(): TestClient.PollResponse; }`); const methods = runner.context.sdkPackage.clients[0].methods; From 01e3093364b4dde979f39d00d014fa09b54ac012 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 16:53:48 +0800 Subject: [PATCH 10/17] add more test case --- .../test/methods/lro.test.ts | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) 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 a97c5dbc52..16c9d3c90a 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -480,16 +480,11 @@ describe("typespec-client-generator-core: long running operation metadata", () = 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); strictEqual(metadata.finalResponse?.envelopeResult, roundtripModel); strictEqual(metadata.finalResponse?.result, roundtripModel); @@ -530,16 +525,11 @@ describe("typespec-client-generator-core: long running operation metadata", () = 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.isUndefined(metadata.finalResponse); }); From 2a0e7eecb9c7baedfc72e235c31183c95b963739 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 17:07:37 +0800 Subject: [PATCH 11/17] update test case to represent more info from the issue --- .../test/methods/lro.test.ts | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) 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 16c9d3c90a..4ddfb28f56 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -385,22 +385,38 @@ describe("typespec-client-generator-core: long running operation metadata", () = }); it("LRO defined in different namespace", async () => { - await runner.compileWithVersionedService(` - model PollResponse { - operationId: string; - status: Azure.Core.Foundations.OperationState; - } - - @pollingOperation(NonService.poll) - @post - @route("/post") - op longRunning(): AcceptedResponse; - - namespace NonService { - @route("/poll") - @get - op poll(): TestClient.PollResponse; - }`); + 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 methods = runner.context.sdkPackage.clients[0].methods; const method = methods.find((m) => m.name === "longRunning"); From 65738a6e83f582fdfc739e838cfaac051e19d26a Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Wed, 20 Nov 2024 17:10:17 +0800 Subject: [PATCH 12/17] refine --- .../typespec-client-generator-core/test/methods/lro.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 4ddfb28f56..ba7dc3ceb1 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -417,9 +417,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = @get op poll(): TestClient.PollResponse; }`); - const methods = runner.context.sdkPackage.clients[0].methods; - - const method = methods.find((m) => m.name === "longRunning"); + const method = runner.context.sdkPackage.clients[0].methods.find( + (m) => m.name === "longRunning", + ); ok(method); strictEqual(method.kind, "lro"); const metadata = method.lroMetadata; From 8eea71d1416d985d977a93c62abbc76d7b6dc119 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 26 Nov 2024 15:21:27 +0800 Subject: [PATCH 13/17] add new usage --- .../src/interfaces.ts | 6 +++ .../src/types.ts | 28 ++++++++--- .../test/methods/lro.test.ts | 50 +++++++++++-------- .../test/test-host.ts | 4 ++ 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index a4518886e1..615fada517 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -742,6 +742,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 211f41e721..c298cb8075 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1595,6 +1595,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)) { @@ -1606,6 +1607,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)); } @@ -1633,20 +1637,32 @@ function updateTypesFromOperation( } } } - const lroMetaData = getLroMetadata(program, operation); + if (lroMetaData && generateConvenient) { - updateUsageOrAccessForLroComponent(lroMetaData.finalResult); + // the final result will be normal output usage. + updateUsageOrAccessForLroComponent(lroMetaData.finalResult, UsageFlags.Output); - updateUsageOrAccessForLroComponent(lroMetaData.finalEnvelopeResult); + // the final envelope result will have LroFinalEnvelope. + updateUsageOrAccessForLroComponent( + lroMetaData.finalEnvelopeResult, + UsageFlags.LroFinalEnvelope, + ); - updateUsageOrAccessForLroComponent(lroMetaData.pollingInfo.responseModel); + // the polling model will have LroPolling. + updateUsageOrAccessForLroComponent( + lroMetaData.pollingInfo.responseModel, + UsageFlags.LroPolling, + ); } return diagnostics.wrap(undefined); - function updateUsageOrAccessForLroComponent(model: Model | "void" | 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, UsageFlags.Output, sdkType)); + diagnostics.pipe(updateUsageOrAccess(context, usage, sdkType)); const access = getAccessOverride(context, operation) ?? "public"; diagnostics.pipe(updateUsageOrAccess(context, access, sdkType)); } 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 ba7dc3ceb1..ed1379462b 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -5,7 +5,7 @@ import { OpenAPITestLibrary } from "@typespec/openapi/testing"; import { notStrictEqual, ok, strictEqual } from "assert"; import { assert, beforeEach, describe, it } from "vitest"; import { SdkHttpOperation, SdkLroServiceMethod, UsageFlags } from "../../src/interfaces.js"; -import { createSdkTestRunner, SdkTestRunner } from "../test-host.js"; +import { createSdkTestRunner, hasFlag, SdkTestRunner } from "../test-host.js"; describe("typespec-client-generator-core: long running operation metadata", () => { let runner: SdkTestRunner; @@ -79,8 +79,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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); @@ -122,8 +123,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusError", ); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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); @@ -171,8 +173,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusExportedUserError", ); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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"); @@ -239,8 +242,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = (m) => m.name === "OperationStatusGenerationResultError", ); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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( @@ -309,8 +313,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = const pollingModel = runner.context.sdkPackage.models.find((m) => m.name === "JobState"); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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); @@ -376,8 +381,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = const pollingModel = runner.context.sdkPackage.models.find((m) => m.name === "JobState"); ok(pollingModel); - strictEqual(pollingModel.usage & UsageFlags.Input, 0); // polling model should not be input - notStrictEqual(pollingModel.usage & UsageFlags.Output, 0); // polling model should be output + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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); @@ -424,8 +430,11 @@ describe("typespec-client-generator-core: long running operation metadata", () = strictEqual(method.kind, "lro"); const metadata = method.lroMetadata; ok(metadata); - ok(method.lroMetadata.pollingStep.responseBody); - notStrictEqual(method.lroMetadata.pollingStep.responseBody.usage & UsageFlags.Output, 0); // the polling model should be output + const pollingModel = metadata.pollingStep.responseBody; + ok(pollingModel); + assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial + 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 }); }); @@ -578,12 +587,11 @@ describe("typespec-client-generator-core: long running operation metadata", () = 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); strictEqual( metadata.pollingStep.responseBody?.name, "ArmOperationStatusResourceProvisioningState", 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; +} From 0d44a8fe34929685141b52e85262e87aa66c5ee4 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 26 Nov 2024 15:57:20 +0800 Subject: [PATCH 14/17] add new test cases --- .../test/methods/lro.test.ts | 237 +++++++++++++++--- 1 file changed, 207 insertions(+), 30 deletions(-) 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 ed1379462b..2e0b686d3c 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -2,9 +2,9 @@ import { FinalStateValue } from "@azure-tools/typespec-azure-core"; import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing"; import { AzureResourceManagerTestLibrary } from "@azure-tools/typespec-azure-resource-manager/testing"; import { OpenAPITestLibrary } from "@typespec/openapi/testing"; -import { notStrictEqual, ok, strictEqual } from "assert"; +import { ok, strictEqual } from "assert"; import { assert, beforeEach, describe, it } from "vitest"; -import { SdkHttpOperation, SdkLroServiceMethod, UsageFlags } from "../../src/interfaces.js"; +import { UsageFlags } from "../../src/interfaces.js"; import { createSdkTestRunner, hasFlag, SdkTestRunner } from "../test-host.js"; describe("typespec-client-generator-core: long running operation metadata", () => { @@ -69,6 +69,13 @@ 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.lroMetadata; ok(metadata); @@ -79,13 +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 LroInitial - 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.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); }); @@ -108,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"); @@ -123,9 +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 LroInitial - 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.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); @@ -164,6 +195,14 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); + 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); @@ -173,9 +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 LroInitial - 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.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"); @@ -184,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", + ); }); }); @@ -233,6 +285,14 @@ describe("typespec-client-generator-core: long running operation metadata", () = inputModel, ); + 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); @@ -242,9 +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 LroInitial - 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.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( @@ -254,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", () => { @@ -306,6 +379,14 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); + 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); @@ -313,9 +394,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = 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 LroInitial - 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 + 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); @@ -374,6 +464,14 @@ describe("typespec-client-generator-core: long running operation metadata", () = "format", ); + 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.location); @@ -381,9 +479,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = 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 LroInitial - 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 + 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); @@ -428,13 +535,31 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); ok(method); strictEqual(method.kind, "lro"); + + 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); const pollingModel = metadata.pollingStep.responseBody; ok(pollingModel); - assert.isTrue(hasFlag(pollingModel.usage, UsageFlags.LroPolling)); // polling model should have the usage of LroInitial - 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.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", + ); }); }); @@ -480,6 +605,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), @@ -489,16 +615,32 @@ 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 - notStrictEqual(roundtripModel.usage & (UsageFlags.Input | UsageFlags.Output), 0); + assert.isTrue( + hasFlag(roundtripModel.usage, UsageFlags.Input | UsageFlags.Output), + "model should be input and output", + ); - strictEqual(method.kind, "lro"); const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.azureAsyncOperation); @@ -510,6 +652,10 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); 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); strictEqual(metadata.finalResponse?.result, roundtripModel); @@ -533,6 +679,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), @@ -543,7 +690,9 @@ describe("typespec-client-generator-core: long running operation metadata", () = "resource", ); - strictEqual(method.kind, "lro"); + const initialResponse = method.response.type; + assert.isUndefined(initialResponse); + const metadata = method.lroMetadata; ok(metadata); strictEqual(metadata.finalStateVia, FinalStateValue.location); @@ -555,6 +704,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); 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); }); @@ -575,13 +736,17 @@ 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"); @@ -592,6 +757,18 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); 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", From bb81da3821b9b07538c9e4d191685d4274a0be0c Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 26 Nov 2024 16:28:22 +0800 Subject: [PATCH 15/17] fix test issues --- .../test/methods/lro.test.ts | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) 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 2e0b686d3c..af9bf3c002 100644 --- a/packages/typespec-client-generator-core/test/methods/lro.test.ts +++ b/packages/typespec-client-generator-core/test/methods/lro.test.ts @@ -380,12 +380,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); 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", - ); + assert.isUndefined(initialResponse); const metadata = method.lroMetadata; ok(metadata); @@ -465,12 +460,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = ); 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", - ); + assert.isUndefined(initialResponse); const metadata = method.lroMetadata; ok(metadata); @@ -537,12 +527,7 @@ describe("typespec-client-generator-core: long running operation metadata", () = strictEqual(method.kind, "lro"); 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", - ); + assert.isUndefined(initialResponse); const metadata = method.lroMetadata; ok(metadata); From 1f53696d106492e93f49de731279cec4a39ada47 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 26 Nov 2024 16:33:27 +0800 Subject: [PATCH 16/17] update changelog --- .chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index bc182afc8b..714c24399c 100644 --- 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 @@ -5,4 +5,5 @@ packages: - "@azure-tools/typespec-client-generator-core" --- -usage and access now properly propagate on polling model, final envelop result of `lroMetadata` +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`. From 4d7efce41a81fe30eea6c485943718f69b9f07b2 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Fri, 29 Nov 2024 14:22:21 +0800 Subject: [PATCH 17/17] update changelog --- .chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 714c24399c..01b4ee5937 100644 --- 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 @@ -1,6 +1,6 @@ --- # Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking -changeKind: fix +changeKind: feature packages: - "@azure-tools/typespec-client-generator-core" ---