Skip to content

Commit

Permalink
Fix the isUnexpected narrowing issue in modular (#2654)
Browse files Browse the repository at this point in the history 
* generate deviceregistry with modular

* Re-generate device registery mgmt

* update tsp

* regen code and build

* Regen the deviceRegistryClient

* Update the api view

* regen code

* fix ci

* fix ci

* remove test case

* update the subset logic for default response

* Update the responses

* Update the interface

* Update the narrowing with necessary conditions

* update

* update

* Update the change for this

* Update packages/typespec-ts/src/modular/helpers/operationHelpers.ts

* Update the changes

* fix ci

* update

* regen smoke test

* remove device case

* fix ci

* regen smoke test

* Regen with res not _result

* Update the generation detail

* regen code

* Resolve comments

* Update the operation name

* revert some changes

* Merge to main

---------

Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com>
Co-authored-by: Jiao Di (MSFT) <80496810+v-jiaodi@users.noreply.github.com>
  • Loading branch information
@MaryGao @kazrael2119 @v-jiaodi
3 people authored Jul 23, 2024
1 parent 454e64d commit 4aff739
Show file tree
Hide file tree
Showing 17 changed files with 417 additions and 332 deletions.
2 changes: 1 addition & 1 deletion packages/rlc-common/src/helpers/schemaHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function isDictionarySchema(
return false;
}

export function isObjectSchema(schema: Schema) {
export function isObjectSchema(schema: Schema): schema is ObjectSchema {
if (schema.type === "object") {
return true;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/rlc-common/src/interfaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ export interface OperationResponse {
operationName: string;
path: string;
responses: ResponseMetadata[];
// Check if the default response is one of superset of non-default responses
isDefaultSupersetOfOthers?: boolean;
}
export interface ResponseMetadata {
statusCode: string;
Expand Down
249 changes: 116 additions & 133 deletions ...test/test/NetworkAnalytics.Management/generated/typespec-ts/src/api/dataProducts/index.ts
View file

Large diffs are not rendered by default.

86 changes: 42 additions & 44 deletions ...ec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/api/dataTypes/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,39 +98,39 @@ export async function _createDeserialize(
throw createRestError(result);
}

result = result as DataTypesCreateLogicalResponse;
const res = result as unknown as DataTypesCreateLogicalResponse;
return {
id: result.body["id"],
name: result.body["name"],
type: result.body["type"],
systemData: !result.body.systemData
id: res.body["id"],
name: res.body["name"],
type: res.body["type"],
systemData: !res.body.systemData
? undefined
: {
createdBy: result.body.systemData?.["createdBy"],
createdByType: result.body.systemData?.["createdByType"],
createdBy: res.body.systemData?.["createdBy"],
createdByType: res.body.systemData?.["createdByType"],
createdAt:
result.body.systemData?.["createdAt"] !== undefined
? new Date(result.body.systemData?.["createdAt"])
res.body.systemData?.["createdAt"] !== undefined
? new Date(res.body.systemData?.["createdAt"])
: undefined,
lastModifiedBy: result.body.systemData?.["lastModifiedBy"],
lastModifiedByType: result.body.systemData?.["lastModifiedByType"],
lastModifiedBy: res.body.systemData?.["lastModifiedBy"],
lastModifiedByType: res.body.systemData?.["lastModifiedByType"],
lastModifiedAt:
result.body.systemData?.["lastModifiedAt"] !== undefined
? new Date(result.body.systemData?.["lastModifiedAt"])
res.body.systemData?.["lastModifiedAt"] !== undefined
? new Date(res.body.systemData?.["lastModifiedAt"])
: undefined,
},
properties: !result.body.properties
properties: !res.body.properties
? undefined
: {
provisioningState: result.body.properties?.["provisioningState"],
state: result.body.properties?.["state"],
stateReason: result.body.properties?.["stateReason"],
provisioningState: res.body.properties?.["provisioningState"],
state: res.body.properties?.["state"],
stateReason: res.body.properties?.["stateReason"],
storageOutputRetention:
result.body.properties?.["storageOutputRetention"],
res.body.properties?.["storageOutputRetention"],
databaseCacheRetention:
result.body.properties?.["databaseCacheRetention"],
databaseRetention: result.body.properties?.["databaseRetention"],
visualizationUrl: result.body.properties?.["visualizationUrl"],
res.body.properties?.["databaseCacheRetention"],
databaseRetention: res.body.properties?.["databaseRetention"],
visualizationUrl: res.body.properties?.["visualizationUrl"],
},
};
}
Expand Down Expand Up @@ -286,39 +286,39 @@ export async function _updateDeserialize(
throw createRestError(result);
}

result = result as DataTypesUpdateLogicalResponse;
const res = result as unknown as DataTypesUpdateLogicalResponse;
return {
id: result.body["id"],
name: result.body["name"],
type: result.body["type"],
systemData: !result.body.systemData
id: res.body["id"],
name: res.body["name"],
type: res.body["type"],
systemData: !res.body.systemData
? undefined
: {
createdBy: result.body.systemData?.["createdBy"],
createdByType: result.body.systemData?.["createdByType"],
createdBy: res.body.systemData?.["createdBy"],
createdByType: res.body.systemData?.["createdByType"],
createdAt:
result.body.systemData?.["createdAt"] !== undefined
? new Date(result.body.systemData?.["createdAt"])
res.body.systemData?.["createdAt"] !== undefined
? new Date(res.body.systemData?.["createdAt"])
: undefined,
lastModifiedBy: result.body.systemData?.["lastModifiedBy"],
lastModifiedByType: result.body.systemData?.["lastModifiedByType"],
lastModifiedBy: res.body.systemData?.["lastModifiedBy"],
lastModifiedByType: res.body.systemData?.["lastModifiedByType"],
lastModifiedAt:
result.body.systemData?.["lastModifiedAt"] !== undefined
? new Date(result.body.systemData?.["lastModifiedAt"])
res.body.systemData?.["lastModifiedAt"] !== undefined
? new Date(res.body.systemData?.["lastModifiedAt"])
: undefined,
},
properties: !result.body.properties
properties: !res.body.properties
? undefined
: {
provisioningState: result.body.properties?.["provisioningState"],
state: result.body.properties?.["state"],
stateReason: result.body.properties?.["stateReason"],
provisioningState: res.body.properties?.["provisioningState"],
state: res.body.properties?.["state"],
stateReason: res.body.properties?.["stateReason"],
storageOutputRetention:
result.body.properties?.["storageOutputRetention"],
res.body.properties?.["storageOutputRetention"],
databaseCacheRetention:
result.body.properties?.["databaseCacheRetention"],
databaseRetention: result.body.properties?.["databaseRetention"],
visualizationUrl: result.body.properties?.["visualizationUrl"],
res.body.properties?.["databaseCacheRetention"],
databaseRetention: res.body.properties?.["databaseRetention"],
visualizationUrl: res.body.properties?.["visualizationUrl"],
},
};
}
Expand Down Expand Up @@ -384,7 +384,6 @@ export async function _$deleteDeserialize(
throw createRestError(result);
}

result = result as DataTypesDeleteLogicalResponse;
return;
}

Expand Down Expand Up @@ -453,7 +452,6 @@ export async function _deleteDataDeserialize(
throw createRestError(result);
}

result = result as DataTypesDeleteDataLogicalResponse;
return;
}

Expand Down
8 changes: 4 additions & 4 deletions packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/api/budgets/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ export async function _createOrReplaceDeserialize(
throw createRestError(result);
}

result = result as BudgetsCreateOrReplaceLogicalResponse;
const res = result as unknown as BudgetsCreateOrReplaceLogicalResponse;
return {
name: result.body["name"],
role: result.body["role"],
id: result.body["id"],
name: res.body["name"],
role: res.body["role"],
id: res.body["id"],
};
}

Expand Down
8 changes: 4 additions & 4 deletions packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/api/widgets/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,11 @@ export async function _createOrReplaceDeserialize(
throw createRestError(result);
}

result = result as WidgetsCreateOrReplaceLogicalResponse;
const res = result as unknown as WidgetsCreateOrReplaceLogicalResponse;
return {
name: result.body["name"],
role: result.body["role"],
id: result.body["id"],
name: res.body["name"],
role: res.body["role"],
id: res.body["id"],
};
}

Expand Down
6 changes: 6 additions & 0 deletions packages/typespec-ts/src/lib.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,12 @@ const libDef = {
messages: {
default: paramMessage`Please note that only compatible additional properties is supported for now. You can enable compatibilityMode to generate compatible additional properties for the model - ${"modelName"}.`
}
},
"default-response-body-type": {
severity: "warning",
messages: {
default: paramMessage`Please note the body type of default response for operation - ${"operationName"} is not a model type.`
}
}
},
emitter: {
Expand Down
97 changes: 65 additions & 32 deletions packages/typespec-ts/src/modular/helpers/operationHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
Operation,
Parameter,
Property,
Response,
Type
} from "../modularCodeModel.js";
import {
Expand All @@ -46,30 +47,55 @@ import {
import { getClassicalLayerPrefix, getOperationName } from "./namingHelpers.js";
import { buildType, isTypeNullable } from "./typeHelpers.js";

function getRLCResponseType(rlcResponse?: OperationResponse) {
function getRLCResponseTypes(rlcResponse?: OperationResponse) {
if (!rlcResponse?.responses) {
return;
}
return rlcResponse?.responses
.map((resp) => {
const baseResponseName = getResponseBaseName(
rlcResponse.operationGroup,
rlcResponse.operationName,
resp.statusCode
);
// Get the information to build the Response Interface
return resp.predefinedName ?? getResponseTypeName(baseResponseName);
})
.map(
(resp) =>
resp.predefinedName ?? getRLCResponseType(resp.statusCode, rlcResponse)
)
.join(" | ");
}

function getRLCLroLogicalResponse(rlcResponse?: OperationResponse) {
const logicalResponse = (rlcResponse?.responses ?? []).filter(
(r) => r.predefinedName && r.predefinedName.endsWith(`LogicalResponse`)
function getRLCResponseType(
statusCode: string,
operationInfo?: OperationResponse
) {
if (!operationInfo) {
return;
}
const baseResponseName = getResponseBaseName(
operationInfo.operationGroup,
operationInfo.operationName,
statusCode
);
// Get the information to build the Response Interface
return getResponseTypeName(baseResponseName);
}

function getNarrowedRLCResponse(
response: Response,
rlcResponse?: OperationResponse
): string | undefined {
if (!rlcResponse) {
return;
}
const statusCode = response.statusCodes;
const names = getRLCResponseTypes(rlcResponse)?.split(" | ");
const lroResponse = names?.filter((n) => n.endsWith(`LogicalResponse`));
const normalResponse = names?.filter(
(n) => n === getRLCResponseType(`${statusCode}`, rlcResponse)
);
return (
// if the response is a LRO response, narrow to the lro logical response
lroResponse?.at(0) ??
// if the default response is a superset of one of other responses, narrow to the normal response
(rlcResponse.isDefaultSupersetOfOthers
? normalResponse?.at(0) ?? "any"
: undefined)
);
return logicalResponse.length > 0
? logicalResponse[0]!.predefinedName!
: "any";
}

export function getSendPrivateFunction(
Expand All @@ -80,7 +106,7 @@ export function getSendPrivateFunction(
): OptionalKind<FunctionDeclarationStructure> {
const parameters = getOperationSignatureParameters(operation, clientType);
const { name } = getOperationName(operation);
const returnType = `StreamableMethod<${getRLCResponseType(
const returnType = `StreamableMethod<${getRLCResponseTypes(
operation.rlcResponse
)}>`;

Expand Down Expand Up @@ -125,7 +151,7 @@ export function getDeserializePrivateFunction(
const parameters: OptionalKind<ParameterDeclarationStructure>[] = [
{
name: "result",
type: getRLCResponseType(operation.rlcResponse)
type: getRLCResponseTypes(operation.rlcResponse)
}
];
// TODO: Support LRO + paging operation
Expand Down Expand Up @@ -187,20 +213,27 @@ export function getDeserializePrivateFunction(
? operation?.lroMetadata?.finalResult
: response.type;
const hasLroSubPath = operation?.lroMetadata?.finalResultPath !== undefined;
// Narrow down the rlc response type to deserialized one
const isNarrowedResponse = getNarrowedRLCResponse(
response,
operation.rlcResponse
);
let deserializePrefix = "result.body";
if (isNarrowedResponse) {
statements.push(`const res = result as unknown as ${isNarrowedResponse};`);
deserializePrefix = "res.body";
}

const deserializedRoot = hasLroSubPath
? `result.body.${operation?.lroMetadata?.finalResultPath}`
: "result.body";
if (isLroOnly) {
const lroLogicalResponse = getRLCLroLogicalResponse(operation.rlcResponse);
statements.push(`result = result as ${lroLogicalResponse};`);
if (hasLroSubPath) {
statements.push(
`if(${deserializedRoot.split(".").join("?.")} === undefined) {
throw createRestError(\`Expected a result in the response at position "${deserializedRoot}"\`, result);
}
`
);
}
? `${deserializePrefix}.${operation?.lroMetadata?.finalResultPath}`
: `${deserializePrefix}`;
if (isLroOnly && hasLroSubPath) {
statements.push(
`if(${deserializedRoot.split(".").join("?.")} === undefined) {
throw createRestError(\`Expected a result in the response at position "${deserializedRoot}"\`, result);
}
`
);
}

const allParents = deserializedType ? getAllAncestors(deserializedType) : [];
Expand All @@ -215,7 +248,7 @@ export function getDeserializePrivateFunction(
response.isBinaryPayload
) {
// TODO: Fix this any cast when implementing handling dict.
statements.push(`return result.body as any`);
statements.push(`return ${deserializedRoot} as any`);
} else if (
deserializedType &&
properties.length > 0 &&
Expand Down
Loading

0 comments on commit 4aff739

Please sign in to comment.