Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce more usages to describe things related with the states of LRO #1887

Merged
merged 25 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d05e51f
remove the workaround since #311 is resolved
ArcturusZhang Nov 19, 2024
c0d712e
Create remove-lro-workaround-2024-10-19-9-4-1.md
ArcturusZhang Nov 20, 2024
8d8fd8c
format
ArcturusZhang Nov 20, 2024
1af0eed
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Nov 20, 2024
83cf322
add some test cases
ArcturusZhang Nov 20, 2024
3edd23b
update test cases
ArcturusZhang Nov 20, 2024
de2c7ba
update changelog
ArcturusZhang Nov 20, 2024
9f98fcf
update test cases
ArcturusZhang Nov 20, 2024
fbe44b2
add a new test case
ArcturusZhang Nov 20, 2024
a19783c
format
ArcturusZhang Nov 20, 2024
01e3093
add more test case
ArcturusZhang Nov 20, 2024
2a0e7ee
update test case to represent more info from the issue
ArcturusZhang Nov 20, 2024
65738a6
refine
ArcturusZhang Nov 20, 2024
99e7840
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Nov 25, 2024
8eea71d
add new usage
ArcturusZhang Nov 26, 2024
0d44a8f
add new test cases
ArcturusZhang Nov 26, 2024
c88c704
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Nov 26, 2024
bb81da3
fix test issues
ArcturusZhang Nov 26, 2024
1f53696
update changelog
ArcturusZhang Nov 26, 2024
286b14e
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Nov 28, 2024
4d7efce
update changelog
ArcturusZhang Nov 29, 2024
0859904
Merge remote-tracking branch 'origin/main' into remove-lro-workaround
ArcturusZhang Nov 29, 2024
413839e
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Dec 3, 2024
3c471d3
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Dec 3, 2024
cec462c
Merge branch 'main' into remove-lro-workaround
ArcturusZhang Dec 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .chronus/changes/remove-lro-workaround-2024-10-19-9-4-1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

usage and access now properly propagate on polling model, final envelop result of `lroMetadata`
32 changes: 13 additions & 19 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1635,27 +1635,21 @@ 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));
}
}
updateUsageOrAccessForLroComponent(lroMetaData.finalResult);

updateUsageOrAccessForLroComponent(lroMetaData.finalEnvelopeResult);

updateUsageOrAccessForLroComponent(lroMetaData.pollingInfo.responseModel);
}
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[]] {
Expand Down
104 changes: 77 additions & 27 deletions packages/typespec-client-generator-core/test/methods/lro.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { ok, strictEqual } from "assert";
import { notStrictEqual, 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", () => {
Expand Down Expand Up @@ -70,7 +70,7 @@ describe("typespec-client-generator-core: long running operation metadata", () =
roundtripModel,
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).lroMetadata;
const metadata = method.lroMetadata;
ok(metadata);
strictEqual(metadata.finalStateVia, FinalStateValue.originalUri);
assert.isUndefined(metadata.finalStep);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -158,7 +162,7 @@ describe("typespec-client-generator-core: long running operation metadata", () =
"format",
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).lroMetadata;
const metadata = method.lroMetadata;
ok(metadata);
strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation);
strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty");
Expand All @@ -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");
Expand Down Expand Up @@ -224,7 +230,7 @@ describe("typespec-client-generator-core: long running operation metadata", () =
inputModel,
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).lroMetadata;
const metadata = method.lroMetadata;
ok(metadata);
strictEqual(metadata.finalStateVia, FinalStateValue.operationLocation);
strictEqual(metadata.finalStep?.kind, "pollingSuccessProperty");
Expand All @@ -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(
Expand Down Expand Up @@ -294,13 +302,15 @@ describe("typespec-client-generator-core: long running operation metadata", () =
"format",
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).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);
Expand Down Expand Up @@ -359,18 +369,64 @@ describe("typespec-client-generator-core: long running operation metadata", () =
"format",
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).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);
});
});

it("LRO defined in different namespace", async () => {
ArcturusZhang marked this conversation as resolved.
Show resolved Hide resolved
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<NoConditionalRequests &
NoRepeatableRequests &
NoClientRequestId>;

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 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", () => {
Expand Down Expand Up @@ -430,23 +486,21 @@ describe("typespec-client-generator-core: long running operation metadata", () =
method.parameters.map((m) => m.type),
roundtripModel,
);
// validate the model should be roundtrip here
notStrictEqual(roundtripModel.usage & (UsageFlags.Input | UsageFlags.Output), 0);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).lroMetadata;
strictEqual(method.kind, "lro");
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);

strictEqual(metadata.finalResponse?.envelopeResult, roundtripModel);
strictEqual(metadata.finalResponse?.result, roundtripModel);
Expand Down Expand Up @@ -480,22 +534,18 @@ describe("typespec-client-generator-core: long running operation metadata", () =
"resource",
);

const metadata = (method as SdkLroServiceMethod<SdkHttpOperation>).lroMetadata;
strictEqual(method.kind, "lro");
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.isUndefined(metadata.finalResponse);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Expand Down
Loading