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

support encode for modular #2006

Merged
merged 19 commits into from
Sep 11, 2023
3 changes: 2 additions & 1 deletion packages/rlc-common/src/buildIndexFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ function generateRLCIndex(file: SourceFile, model: RLCModel) {
hasMultiCollection(model) ||
hasSsvCollection(model) ||
hasPipeCollection(model) ||
hasTsvCollection(model)
hasTsvCollection(model) ||
hasCsvCollection(model)
) {
file.addExportDeclarations([
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,12 @@ export function _addOrUpdateBlockItemsSend(
)
.post({
...operationOptionsToRequestParameters(options),
body: { blockItems: blockItems },
body: {
blockItems: (blockItems ?? []).map((p) => ({
description: p["description"],
text: p["text"],
})),
},
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export function _publishCloudEventSend(
? uint8ArrayToString(event["dataBase64"], "base64")
: undefined,
type: event["type"],
time:
event["time"] !== undefined ? new Date(event["time"]) : undefined,
time: event["time"]?.toISOString(),
Copy link
Contributor

@MaryGao MaryGao Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice the typespec type here is utcDateTime but we generate to toISOString

https://github.com/Azure/autorest.typescript/blob/1235e0ba4762ac1171f5777b50eafb1b7e43a8d0/packages/typespec-test/test/eventgrid_modular/spec/main.tsp#L64-L65C24

Not sure it is possible to add some UTs to cover typespec type to modular type:

/**
 * A date on a calendar without a time zone, e.g. "April 10th"
 */
scalar plainDate;

/**
 * A time on a clock without a time zone, e.g. "3:00 am"
 */
scalar plainTime;

/**
 * An instant in coordinated universal time (UTC)"
 */
scalar utcDateTime;

/**
 * A date and time in a particular time zone, e.g. "April 10th at 3:00am in PST"
 */
scalar offsetDateTime;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect typespec compiler does not provide any format info to these types, but let me check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typescript date.toUTCString() is "Fri, 08 Sep 2023 08:50:03 GMT" and date.toISOString() is "2023-09-08T08:42:49.439Z"
therefore use toISOString for utcDateTime makes more sense. and toUTCString for offsetDateTime makes more sense to me.

Copy link
Contributor

@MaryGao MaryGao Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it makes sense to me also. Just checking with Tim here microsoft/typespec#1899 (comment)

Copy link
Contributor

@MaryGao MaryGao Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not the same
image

specversion: event["specversion"],
dataschema: event["dataschema"],
datacontenttype: event["datacontenttype"],
Expand Down Expand Up @@ -123,7 +122,7 @@ export function _publishCloudEventsSend(
? uint8ArrayToString(p["dataBase64"], "base64")
: undefined,
type: p["type"],
time: p["time"] !== undefined ? new Date(p["time"]) : undefined,
time: p["time"]?.toISOString(),
specversion: p["specversion"],
dataschema: p["dataschema"],
datacontenttype: p["datacontenttype"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,12 @@ export function _listMetricsSend(
metricNamespace: options?.metricNamespace,
timespan: options?.timespan,
},
body: { filters: options?.filters },
body: {
filters: (options?.filters ?? []).map((p) => ({
name: p["name"],
values: p["values"],
})),
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you point to me where is the fix code?

});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,29 @@ export function _getChatCompletionsSend(
.post({
...operationOptionsToRequestParameters(options),
body: {
messages: messages,
functions: options?.functions,
messages: (messages ?? []).map((p) => ({
role: p["role"],
content: p["content"],
name: p["name"],
function_call: !p.functionCall
? undefined
: {
name: p.functionCall?.["name"],
arguments: p.functionCall?.["arguments"],
},
context: !p.context
? undefined
: {
messages: !p.context?.messages
? undefined
: (p.context?.messages as any),
},
})),
functions: (options?.functions ?? []).map((p) => ({
name: p["name"],
description: p["description"],
parameters: p["parameters"],
})),
function_call: options?.functionCall,
max_tokens: options?.maxTokens,
temperature: options?.temperature,
Expand All @@ -256,7 +277,10 @@ export function _getChatCompletionsSend(
frequency_penalty: options?.frequencyPenalty,
stream: options?.stream,
model: options?.model,
dataSources: options?.dataSources,
dataSources: (options?.dataSources ?? []).map((p) => ({
type: p["type"],
parameters: p["parameters"],
})),
},
});
}
Expand Down Expand Up @@ -400,8 +424,29 @@ export function _getChatCompletionsWithAzureExtensionsSend(
.post({
...operationOptionsToRequestParameters(options),
body: {
messages: messages,
functions: options?.functions,
messages: (messages ?? []).map((p) => ({
role: p["role"],
content: p["content"],
name: p["name"],
function_call: !p.functionCall
? undefined
: {
name: p.functionCall?.["name"],
arguments: p.functionCall?.["arguments"],
},
context: !p.context
? undefined
: {
messages: !p.context?.messages
? undefined
: (p.context?.messages as any),
},
})),
functions: (options?.functions ?? []).map((p) => ({
name: p["name"],
description: p["description"],
parameters: p["parameters"],
})),
function_call: options?.functionCall,
max_tokens: options?.maxTokens,
temperature: options?.temperature,
Expand All @@ -414,7 +459,10 @@ export function _getChatCompletionsWithAzureExtensionsSend(
frequency_penalty: options?.frequencyPenalty,
stream: options?.stream,
model: options?.model,
dataSources: options?.dataSources,
dataSources: (options?.dataSources ?? []).map((p) => ({
type: p["type"],
parameters: p["parameters"],
})),
},
});
}
Expand Down
81 changes: 63 additions & 18 deletions packages/typespec-ts/src/modular/buildCodeModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
isStringType,
getPropertyType,
isNumericType,
getFormat,
getMinItems,
getMaxItems,
listServices,
Expand Down Expand Up @@ -155,7 +154,8 @@ function isSimpleType(
getMaxValue,
getMinLength,
getMaxLength,
getPattern
getPattern,
getEncode
];
for (const func of funcs) {
if (func(program, type)) {
Expand Down Expand Up @@ -277,8 +277,7 @@ function getType(
options: { disableEffectiveModel?: boolean } = {}
): any {
// don't cache simple type(string, int, etc) since decorators may change the result
const enableCache =
!isSimpleType(context.program, type) && !isEmptyModel(type);
const enableCache = !isSimpleType(context.program, type);
const effectiveModel =
!options.disableEffectiveModel &&
(type.kind === "Model" || type.kind === "Union")
Expand All @@ -298,6 +297,10 @@ function getType(
newValue = emitType(context, type);
}

if (type.kind === "ModelProperty" || type.kind === "Scalar") {
newValue = applyEncoding(context.program, type, newValue);
}

if (enableCache) {
typesMap.set(effectiveModel, newValue);
if (type.kind === "Union") {
Expand Down Expand Up @@ -335,6 +338,7 @@ type ParamBase = {
addedOn: string | undefined;
clientName: string;
inOverload: boolean;
format?: string;
};
function emitParamBase(
program: Program,
Expand All @@ -344,12 +348,15 @@ function emitParamBase(
let name: string;
let description: string = "";
let addedOn: string | undefined;
let format: string | undefined;

if (parameter.kind === "ModelProperty") {
const newParameter = applyEncoding(program, parameter, parameter);
optional = parameter.optional;
name = parameter.name;
description = getDocStr(program, parameter);
addedOn = getAddedOnVersion(program, parameter);
format = newParameter.format;
} else {
optional = false;
name = "body";
Expand All @@ -360,7 +367,8 @@ function emitParamBase(
description,
addedOn,
clientName: applyCasing(name, { casing: CASING }),
inOverload: false
inOverload: false,
format
};
}

Expand Down Expand Up @@ -428,7 +436,6 @@ function emitBodyParameter(
const type = getType(context, getBodyType(context.program, httpOperation), {
disableEffectiveModel: true
});

if (type.type === "model" && type.name === "") {
type.name = capitalize(httpOperation.operation.name) + "Request";
}
Expand Down Expand Up @@ -466,10 +473,10 @@ function emitParameter(
const paramMap: any = {
restApiName: parameter.name,
location: parameter.type,
type: type,
type,
implementation: implementation,
skipUrlEncoding: parameter.type === "endpointPath",
format: (parameter as any).format
format: (parameter as any).format ?? base.format
};

if (paramMap.type.type === "constant") {
Expand Down Expand Up @@ -828,6 +835,7 @@ function emitProperty(
context: SdkContext,
property: ModelProperty
): Record<string, any> {
const newProperty = applyEncoding(context.program, property, property);
let clientDefaultValue = undefined;
const propertyDefaultKind = property.default?.kind;
if (
Expand Down Expand Up @@ -860,7 +868,8 @@ function emitProperty(
addedOn: getAddedOnVersion(context.program, property),
readonly:
isReadOnly(context.program, property) || isKey(context.program, property),
clientDefaultValue: clientDefaultValue
clientDefaultValue: clientDefaultValue,
format: newProperty.format
};
}

Expand Down Expand Up @@ -1024,9 +1033,10 @@ function emitStdScalar(
program: Program,
scalar: Scalar & { name: IntrinsicScalarName }
): Record<string, any> {
const newScalar = applyEncoding(program, scalar, scalar);
switch (scalar.name) {
case "bytes":
return { type: "byte-array", format: getEncode(program, scalar) };
return { type: "byte-array", format: newScalar.format };
case "int8":
case "int16":
case "int32":
Expand All @@ -1050,24 +1060,61 @@ function emitStdScalar(
case "plainDate":
return { type: "date" };
case "utcDateTime":
return { type: "datetime", format: "date-time" };
return { type: "datetime", format: newScalar.format };
case "plainTime":
return { type: "time" };
case "duration":
return { type: "duration" };
return { type: "duration", format: newScalar.format };
case "numeric":
return {}; // Waiting on design for more precise type https://github.com/microsoft/cadl/issues/1260
default:
return {};
}
}

function applyEncoding(
program: Program,
typespecType: Scalar | ModelProperty,
target: any = {}
) {
const encodeData = getEncode(program, typespecType);
if (encodeData) {
const newTarget = { ...target };
const newType = emitScalar(program, encodeData.type);
// newTarget["type"] = newType["type"];
// If the target already has a format it takes priority. (e.g. int32)
newTarget["format"] = mergeFormatAndEncoding(
newTarget.format,
encodeData.encoding,
newType["format"]
);
return newTarget;
}
return target;
}

function mergeFormatAndEncoding(
format: string | undefined,
encoding: string,
encodeAsFormat: string | undefined
): string {
switch (format) {
case undefined:
return encodeAsFormat ?? encoding;
case "date-time":
return encoding;
case "duration":
default:
return encodeAsFormat ?? encoding;
}
}

function applyIntrinsicDecorators(
program: Program,
type: Scalar | ModelProperty,
result: any
): Record<string, any> {
const newResult = { ...result };
let newResult = { ...result };
const docStr = getDoc(program, type);
const isString = isStringType(program, getPropertyType(type));
const isNumeric = isNumericType(program, getPropertyType(type));
Expand All @@ -1076,10 +1123,7 @@ function applyIntrinsicDecorators(
newResult.description = docStr;
}

const formatStr = getFormat(program, type);
if (isString && !result.format && formatStr) {
newResult.format = formatStr;
}
newResult = applyEncoding(program, type, newResult);

const pattern = getPattern(program, type);
if (isString && !result.pattern && pattern) {
Expand Down Expand Up @@ -1120,7 +1164,8 @@ function applyIntrinsicDecorators(

function emitScalar(program: Program, scalar: Scalar): Record<string, any> {
let result: Record<string, any> = {};
if (program.checker.isStdType(scalar)) {
const isStd = program.checker.isStdType(scalar);
if (isStd) {
result = emitStdScalar(program, scalar);
} else if (scalar.baseScalar) {
result = emitScalar(program, scalar.baseScalar);
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-ts/src/modular/buildOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export function buildOperationOptions(
return {
docs: getDocsFromDescription(p.description),
hasQuestionToken: true,
...buildType(p.clientName, p.type)
...buildType(p.clientName, p.type, p.format)
};
})
});
Expand Down
6 changes: 3 additions & 3 deletions packages/typespec-ts/src/modular/emitModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function buildModels(

for (const model of models) {
const properties = model.properties ?? [];
const typeMetadata = getType(model);
const typeMetadata = getType(model, model.format);
let typeName = typeMetadata.name;
if (typeMetadata.modifier === "Array") {
typeName = `${typeName}[]`;
Expand Down Expand Up @@ -77,7 +77,7 @@ export function buildModels(
docs: getDocsFromDescription(model.description),
extends: [] as string[],
properties: properties.map((p) => {
const propertyMetadata = getType(p.type);
const propertyMetadata = getType(p.type, p.format);
let propertyTypeName = propertyMetadata.name;
if (isAzureCoreErrorSdkType(p.type)) {
propertyTypeName = isAzureCoreErrorSdkType(p.type)
Expand All @@ -98,7 +98,7 @@ export function buildModels(
};
model.type === "model"
? model.parents?.forEach((p) =>
modelInterface.extends.push(getType(p).name)
modelInterface.extends.push(getType(p, p.format).name)
)
: undefined;
modelsFile.addInterface(modelInterface);
Expand Down
Loading
Loading