diff --git a/.chronus/changes/async-header-result-2024-7-22-23-19-8.md b/.chronus/changes/async-header-result-2024-7-22-23-19-8.md new file mode 100644 index 0000000000..7d79d07522 --- /dev/null +++ b/.chronus/changes/async-header-result-2024-7-22-23-19-8.md @@ -0,0 +1,9 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@azure-tools/typespec-azure-core" + - "@azure-tools/typespec-azure-resource-manager" +--- + +Fix #1180 Return StatusMonitor result field for non-resource PUT operations in getLroMetadata.finalResult diff --git a/packages/typespec-azure-core/src/lro-helpers.ts b/packages/typespec-azure-core/src/lro-helpers.ts index d4d1761143..7ab740cb11 100644 --- a/packages/typespec-azure-core/src/lro-helpers.ts +++ b/packages/typespec-azure-core/src/lro-helpers.ts @@ -117,6 +117,9 @@ export interface OperationReference { parameterMap?: Map; parameters?: Map; + + /** headers linking to the operation */ + link?: OperationLink; } /** @@ -310,15 +313,18 @@ export function getLroMetadata(program: Program, operation: Operation): LroMetad if (context === undefined) return undefined; processFinalReference(program, operation, context); processFinalLink(program, operation, context); - const nextReference: NextOperationReference | undefined = processStatusMonitorReference( - program, - operation, - context - ); + const nextReference: + | NextOperationReference + | (NextOperationLink & { operation: Operation }) + | undefined = processStatusMonitorReference(program, operation, context); if (nextReference !== undefined && nextReference.responseModel.kind === "Model") { context.statusMonitorStep = nextReference; - processFinalReference(program, nextReference.target.operation, context); - processFinalLink(program, nextReference.target.operation, context); + const linkedOperation = + nextReference.kind === "nextOperationLink" + ? nextReference.operation + : nextReference.target.operation; + processFinalReference(program, linkedOperation, context); + processFinalLink(program, linkedOperation, context); context.pollingStep = getPollingStep(program, nextReference.responseModel, context); return createLroMetadata(program, operation, context); } @@ -463,8 +469,10 @@ function createOperationLink(program: Program, modelProperty: ModelProperty): Op }; } -function createOperationReference(metadata: OperationLinkMetadata): OperationReference | undefined { - if (!metadata.parameterMap) return undefined; +function createOperationReferenceOrLink( + metadata: OperationLinkMetadata +): OperationReference | OperationLink | undefined { + if (!metadata.parameterMap && !metadata.link) return undefined; const map = new Map(); if (metadata.parameterMap) { for (const [name, parameters] of metadata.parameterMap) { @@ -487,6 +495,13 @@ function createOperationReference(metadata: OperationLinkMetadata): OperationRef parameters: metadata.parameterMap, }; } + if (metadata.link) { + return { + kind: "link", + location: metadata.link.location, + property: metadata.link.property, + }; + } return undefined; } @@ -554,9 +569,6 @@ function getLogicalResourceOperation( case "delete": resultOp = "delete"; break; - case "put": - resultOp = "createOrReplace"; - break; default: return undefined; } @@ -602,6 +614,12 @@ function getFinalStateVia( break; case "nextOperationReference": finalState = FinalStateValue.customOperationReference; + if (context.statusMonitorStep.target.link?.location === "ResponseHeader") { + finalState = getLroStatusFromHeaderProperty( + program, + context.statusMonitorStep.target.link.property + ); + } } } else { finalState = getStatusFromLinkOrReference(program, operation, context.finalStep.target); @@ -700,7 +718,6 @@ function getPollingStep( PollingOperationKey ); } - if (context.pollingOperationLink?.parameterMap === undefined) return undefined; const statusMonitorOverride = context.pollingOperationLink?.result?.statusMonitor; if (statusMonitorOverride !== undefined && statusMonitorOverride.monitorType !== undefined) { info = { @@ -754,6 +771,8 @@ function getStatusFromLinkOrReference( finalState = FinalStateValue.customOperationReference; if (isMatchingGetOperation(program, sourceOperation, target.operation)) { finalState = FinalStateValue.originalUri; + } else if (target.link !== undefined && target.link.location === "ResponseHeader") { + finalState = getLroStatusFromHeaderProperty(program, target.link.property); } } break; @@ -802,6 +821,7 @@ function GetStatusMonitorInfoFromOperation( program: Program, operation: HttpOperation ): StatusMonitorInfo | undefined { + if (operation.verb === "get" || operation.verb === "head") return undefined; const models = filterResponseModels( operation, (model) => @@ -971,16 +991,31 @@ function processFinalReference(program: Program, operation: Operation, context: if (context.finalStep !== undefined) return; // looks for operation marked with @finalOperation const link = getOperationLink(program, operation, "final"); - if (link === undefined || link.parameterMap === undefined || link.result?.type === undefined) + if ( + link === undefined || + link.result?.type === undefined || + (link.link === undefined && link.parameterMap === undefined) + ) return; context.finalOperationLink = link; - const reference = createOperationReference(link); + const reference = createOperationReferenceOrLink(link); if (reference === undefined) return; - context.finalStep = { - kind: "finalOperationReference", - responseModel: link.result?.type, - target: reference, - }; + switch (reference.kind) { + case "reference": + context.finalStep = { + kind: "finalOperationReference", + responseModel: link.result?.type, + target: reference, + }; + break; + case "link": + context.finalStep = { + kind: "finalOperationLink", + responseModel: link.result?.type, + target: reference, + }; + break; + } } function createStatusMonitorPollingData(data: StatusMonitorMetadata): StatusMonitorInfo { @@ -1063,7 +1098,7 @@ function processStatusMonitorReference( program: Program, referencedOperation: Operation, context: LroContext -): NextOperationReference | undefined { +): NextOperationReference | (NextOperationLink & { operation: Operation }) | undefined { const references: Map | undefined = getOperationLinks( program, referencedOperation @@ -1073,19 +1108,29 @@ function processStatusMonitorReference( const pollingData: OperationLinkMetadata | undefined = references.get(PollingOperationKey); if (pollingData === undefined) return undefined; context.pollingOperationLink = pollingData; - const pollingReference = createOperationReference(pollingData); + const pollingReference = createOperationReferenceOrLink(pollingData); if (pollingReference === undefined) return undefined; context.statusMonitorInfo = pollingData.result?.statusMonitor; const finalData: OperationLinkMetadata | undefined = references.get(FinalOperationKey); if (context.finalStep === undefined && finalData !== undefined) { - const finalReference = createOperationReference(finalData); + const finalReference = createOperationReferenceOrLink(finalData); const finalModel = finalData?.result?.type; if (finalReference !== undefined && finalModel !== undefined) { - context.finalStep = { - kind: "finalOperationReference", - responseModel: finalModel, - target: finalReference, - }; + switch (finalReference.kind) { + case "reference": + context.finalStep = { + kind: "finalOperationReference", + responseModel: finalModel, + target: finalReference, + }; + break; + case "link": + context.finalStep = { + kind: "finalOperationLink", + responseModel: finalModel, + target: finalReference, + }; + } } } if ( @@ -1102,11 +1147,21 @@ function processStatusMonitorReference( } const responseModel = pollingData.result?.type; if (responseModel === undefined) return undefined; - return { - kind: "nextOperationReference", - responseModel: responseModel, - target: pollingReference, - }; + switch (pollingReference.kind) { + case "reference": + return { + kind: "nextOperationReference", + responseModel: responseModel, + target: pollingReference, + }; + case "link": + return { + kind: "nextOperationLink", + responseModel: responseModel, + target: pollingReference, + operation: pollingData.linkedOperation, + }; + } } function resolveOperationLocation( diff --git a/packages/typespec-azure-core/src/lro-info.ts b/packages/typespec-azure-core/src/lro-info.ts index ce0868f0b3..8264e145c4 100644 --- a/packages/typespec-azure-core/src/lro-info.ts +++ b/packages/typespec-azure-core/src/lro-info.ts @@ -166,7 +166,7 @@ export function getLroOperationInfo( for (const response of targetResponses) { visitResponse(program, response, (model) => { if (!isErrorType(model) && resultModel === undefined) { - resultModel = model; + resultModel = getEffectiveModelType(program, model, (p) => !isMetadata(program, p)); } }); } diff --git a/packages/typespec-azure-core/test/operations.test.ts b/packages/typespec-azure-core/test/operations.test.ts index fbd381ea2b..35b3356b2e 100644 --- a/packages/typespec-azure-core/test/operations.test.ts +++ b/packages/typespec-azure-core/test/operations.test.ts @@ -960,6 +960,154 @@ describe("typespec-azure-core: operation templates", () => { deepStrictEqual(metadata.finalResultPath, "result"); }); + it("Gets Lro for non-resource PUT with polling reference", async () => { + const [_, metadata] = await compileLroOperation( + ` + /** The created job */ +model Job { + /** job id */ + @path + @key + @visibility("read") + id: uuid; + + /** job name */ + name: string; +} + +/** request to create a job */ +model StartJobRequest { + /** Name of the job */ + name: string; + + /** Job instructions */ + instructions: string[]; +} + +model JobStatus is Azure.Core.Foundations.OperationStatus; + +#suppress "@azure-tools/typespec-azure-core/use-standard-operations" "Non-resource operation" +@route("/jobs/{id}") +op getJobStatus is Foundations.GetOperationStatus< + Rest.Resource.KeysOf, + Job +>; + +alias JobLroResponse = { + @header("Operation-Location") operationLocation: url; + ...Azure.Core.Foundations.OperationStatus; +}; + +/** Start a job */ +#suppress "@azure-tools/typespec-azure-core/use-standard-operations" "Non-resource operation" +@pollingOperation(getJobStatus) +@route("/startJob/") +@put +op startJobAsync( + ...Azure.Core.Foundations.ApiVersionParameter, + + /** body */ + @body _: StartJobRequest, +): (CreatedResponse & JobLroResponse) | (OkResponse & + JobLroResponse) | Azure.Core.Foundations.ErrorResponse; +`, + "startJobAsync" + ); + + ok(metadata); + deepStrictEqual(metadata.statusMonitorStep?.kind, "nextOperationLink"); + deepStrictEqual(metadata.statusMonitorStep?.responseModel.name, "OperationStatus"); + + deepStrictEqual(metadata.pollingInfo.kind, "pollingOperationStep"); + deepStrictEqual(metadata.pollingInfo.responseModel.name, "OperationStatus"); + deepStrictEqual(metadata.pollingInfo.resultProperty?.name, "result"); + + deepStrictEqual(metadata.finalStateVia, "operation-location"); + deepStrictEqual(metadata.logicalResult.name, "Job"); + deepStrictEqual(metadata.envelopeResult.name, "OperationStatus"); + deepStrictEqual(metadata.logicalPath, "result"); + + deepStrictEqual((metadata.finalResult as Model)?.name, "Job"); + deepStrictEqual((metadata.finalEnvelopeResult as Model)?.name, "OperationStatus"); + deepStrictEqual(metadata.finalResultPath, "result"); + }); + + it("Gets Lro for custom PUT with polling reference", async () => { + const [_, metadata] = await compileLroOperation( + ` + /** The created job */ + @resource("jobs") +model Job { + /** job id */ + @path + @key + @visibility("read") + name: string; + + /** Job instructions */ + instructions: string[]; +} + +/** request to create a job */ +model StartJobRequest { + /** Name of the job */ + name: string; + + /** Job instructions */ + instructions: string[]; +} + +model JobStatus is Azure.Core.Foundations.OperationStatus; + +#suppress "@azure-tools/typespec-azure-core/use-standard-operations" "Non-resource operation" +@route("/jobs/status/{name}") +op getJobStatus is Foundations.GetOperationStatus< + Rest.Resource.KeysOf, + Job +>; + +op read is StandardResourceOperations.ResourceRead; + +alias JobLroResponse = { + @header("Operation-Location") operationLocation: string; + ...Azure.Core.Foundations.OperationStatus; +}; + +/** Start a job */ +#suppress "@azure-tools/typespec-azure-core/use-standard-operations" "test" +@finalOperation(read) +@pollingOperation(getJobStatus) +@createsOrReplacesResource(Job) +@route("/jobs/{name}") +@put +op createJob( + ...Azure.Core.Foundations.ApiVersionParameter, + @path name: string, + @bodyRoot body: Job, +): (CreatedResponse & JobLroResponse) | (OkResponse & + JobLroResponse) | Azure.Core.Foundations.ErrorResponse; +`, + "createJob" + ); + + ok(metadata); + deepStrictEqual(metadata.statusMonitorStep?.kind, "nextOperationLink"); + deepStrictEqual(metadata.statusMonitorStep?.responseModel.name, "OperationStatus"); + + deepStrictEqual(metadata.pollingInfo.kind, "pollingOperationStep"); + deepStrictEqual(metadata.pollingInfo.responseModel.name, "OperationStatus"); + deepStrictEqual(metadata.pollingInfo.resultProperty?.name, "result"); + + deepStrictEqual(metadata.finalStateVia, "original-uri"); + deepStrictEqual(metadata.logicalResult.name, "Job"); + deepStrictEqual(metadata.envelopeResult.name, "OperationStatus"); + deepStrictEqual(metadata.logicalPath, undefined); + + deepStrictEqual((metadata.finalResult as Model)?.name, "Job"); + deepStrictEqual((metadata.finalEnvelopeResult as Model)?.name, "Job"); + deepStrictEqual(metadata.finalResultPath, undefined); + }); + it("Gets Lro for standard Async Delete", async () => { const [_, metadata, runner] = await compileLroOperation( `@test op delete is Azure.Core.LongRunningResourceDelete;` @@ -1462,6 +1610,7 @@ describe("typespec-azure-core: operation templates", () => { } @route("/simpleWidgets/{id}") + @createsOrReplacesResource(SimpleWidget) @put op createWidget(@path id: string, body: SimpleWidget) : SimpleWidget | { @statusCode statusCode: 201; @@ -1509,6 +1658,7 @@ describe("typespec-azure-core: operation templates", () => { } @route("/simpleWidgets/{id}") + @createsOrUpdatesResource(SimpleWidget) @put op createWidget(@path id: string, body: SimpleWidget) : SimpleWidget | { @statusCode statusCode: 201; @@ -1991,8 +2141,8 @@ describe("typespec-azure-core: operation templates", () => { ok(metadata.statusMonitorStep); deepStrictEqual(metadata.statusMonitorStep.target.kind, "link"); - deepStrictEqual((metadata.statusMonitorStep.target as any).location, "ResponseHeader"); - deepStrictEqual((metadata.statusMonitorStep.target as any).property.name, "opLink"); + deepStrictEqual((metadata.statusMonitorStep.target as any)?.location, "ResponseHeader"); + deepStrictEqual((metadata.statusMonitorStep.target as any)?.property.name, "opLink"); ok(metadata.pollingInfo); deepStrictEqual(metadata.pollingInfo.responseModel.name, "PollingStatus"); @@ -2026,6 +2176,7 @@ describe("typespec-azure-core: operation templates", () => { @pollingOperation(getStatus) @route("/simpleWidgets/{id}") + @createsOrReplacesResource(SimpleWidget) @put op createWidget(@path id: string, body: SimpleWidget) : SimpleWidget | { @statusCode statusCode: 201; @@ -2050,8 +2201,8 @@ describe("typespec-azure-core: operation templates", () => { ok(metadata.statusMonitorStep); deepStrictEqual(metadata.statusMonitorStep.target.kind, "link"); - deepStrictEqual((metadata.statusMonitorStep.target as any).location, "ResponseHeader"); - deepStrictEqual((metadata.statusMonitorStep.target as any).property.name, "opLink"); + deepStrictEqual((metadata.statusMonitorStep.target as any)?.location, "ResponseHeader"); + deepStrictEqual((metadata.statusMonitorStep.target as any)?.property.name, "opLink"); ok(metadata.pollingInfo); deepStrictEqual(metadata.pollingInfo.responseModel.name, "PollingStatus"); diff --git a/packages/typespec-azure-resource-manager/src/operations.ts b/packages/typespec-azure-resource-manager/src/operations.ts index b5640223ab..0a0b1f6eba 100644 --- a/packages/typespec-azure-resource-manager/src/operations.ts +++ b/packages/typespec-azure-resource-manager/src/operations.ts @@ -8,7 +8,16 @@ import { Program, } from "@typespec/compiler"; import { getHttpOperation, HttpOperation } from "@typespec/http"; -import { $actionSegment, getActionSegment, getParentResource, getSegment } from "@typespec/rest"; +import { + $actionSegment, + $createsOrReplacesResource, + $deletesResource, + $readsResource, + $updatesResource, + getActionSegment, + getParentResource, + getSegment, +} from "@typespec/rest"; import { ArmResourceActionDecorator, ArmResourceCollectionActionDecorator, @@ -147,6 +156,7 @@ export const $armResourceRead: ArmResourceReadDecorator = ( target: Operation, resourceType: Model ) => { + context.call($readsResource, target, resourceType); setResourceLifecycleOperation(context, target, resourceType, "read", "@armResourceRead"); }; @@ -155,6 +165,7 @@ export const $armResourceCreateOrUpdate: ArmResourceCreateOrUpdateDecorator = ( target: Operation, resourceType: Model ) => { + context.call($createsOrReplacesResource, target, resourceType); setResourceLifecycleOperation( context, target, @@ -169,6 +180,7 @@ export const $armResourceUpdate: ArmResourceUpdateDecorator = ( target: Operation, resourceType: Model ) => { + context.call($updatesResource, target, resourceType); setResourceLifecycleOperation(context, target, resourceType, "update", "@armResourceUpdate"); }; @@ -177,6 +189,7 @@ export const $armResourceDelete: ArmResourceDeleteDecorator = ( target: Operation, resourceType: Model ) => { + context.call($deletesResource, target, resourceType); setResourceLifecycleOperation(context, target, resourceType, "delete", "@armResourceDelete"); };