Bypass autocorrect to type: object in some cases (#4731)

This commit is contained in:
Mike Kistler 2023-05-02 08:38:07 -07:00 коммит произвёл GitHub
Родитель 42cc4e7493
Коммит 73f476d113
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
7 изменённых файлов: 250 добавлений и 23 удалений

Просмотреть файл

@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@autorest/extension-base",
"comment": "Bypass autocorrect to type: object in some cases",
"type": "patch"
}
],
"packageName": "@autorest/extension-base"
}

Просмотреть файл

@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@autorest/modelerfour",
"comment": "Bypass autocorrect to type: object in some cases",
"type": "patch"
}
],
"packageName": "@autorest/modelerfour"
}

Просмотреть файл

@ -1145,17 +1145,29 @@ export class ModelerFour {
}
if (schema.allOf || schema.anyOf || schema.oneOf) {
// if the model has properties, then we're going to assume they meant to say JsonType.object
// but we're going to warn them anyway.
this.session.warning(
`The schema '${
schema?.["x-ms-metadata"]?.name || name
}' with an undefined type and 'allOf'/'anyOf'/'oneOf' is a bit ambiguous. This has been auto-corrected to 'type:object'`,
["Modeler", "MissingType"],
schema,
);
schema.type = OpenAPI.JsonType.Object;
break;
// The schema does not have properties or additionalProperties, but it does have allOf/anyOf/oneOf.
// The prior logic auto-corrected this to type: object, but that's not always appropriate.
// Check the child schemas and bypass the auto-correct if any are clearly not type: object.
// Return true if the schema has an explicit type that is not type: object.
const notTypeObject = (e: Refable<OpenAPI.Schema>): boolean => {
const s = this.resolve(e).instance;
return !!s.type && s.type !== OpenAPI.JsonType.Object;
};
let bypassAutoCorrect = schema.allOf && schema.allOf.some(notTypeObject);
bypassAutoCorrect ||= schema.anyOf && schema.anyOf.some(notTypeObject);
bypassAutoCorrect ||= schema.oneOf && schema.oneOf.some(notTypeObject);
if (!bypassAutoCorrect) {
this.session.warning(
`The schema '${
schema?.["x-ms-metadata"]?.name || name
}' with an undefined type and 'allOf'/'anyOf'/'oneOf' is a bit ambiguous. This has been auto-corrected to 'type:object'`,
["Modeler", "MissingType"],
schema,
);
schema.type = OpenAPI.JsonType.Object;
break;
}
}
{

Просмотреть файл

@ -274,17 +274,29 @@ export class QualityPreChecker {
}
if (schema.allOf || schema.anyOf || schema.oneOf) {
// if the model has properties, then we're going to assume they meant to say JsonType.object
// but we're going to warn them anyway.
this.session.warning(
`The schema '${
schema?.["x-ms-metadata"]?.name || name
}' with an undefined type and 'allOf'/'anyOf'/'oneOf' is a bit ambiguous. This has been auto-corrected to 'type:object'`,
["PreCheck", "SchemaMissingType"],
schema,
);
schema.type = JsonType.Object;
break;
// The schema does not have properties or additionalProperties, but it does have allOf/anyOf/oneOf.
// The prior logic auto-corrected this to type: object, but that's not always appropriate.
// Check the child schemas and bypass the auto-correct if any are clearly not type: object.
// Return true if the schema has an explicit type that is not type: object.
const notTypeObject = (e: Refable<Schema>): boolean => {
const s = this.resolve(e).instance;
return !!s.type && s.type !== JsonType.Object;
};
let bypassAutoCorrect = schema.allOf && schema.allOf.some(notTypeObject);
bypassAutoCorrect ||= schema.anyOf && schema.anyOf.some(notTypeObject);
bypassAutoCorrect ||= schema.oneOf && schema.oneOf.some(notTypeObject);
if (!bypassAutoCorrect) {
this.session.warning(
`The schema '${
schema?.["x-ms-metadata"]?.name || name
}' with an undefined type and 'allOf'/'anyOf'/'oneOf' is a bit ambiguous. This has been auto-corrected to 'type:object'`,
["PreCheck", "SchemaMissingType"],
schema,
);
schema.type = JsonType.Object;
break;
}
}
break;
}

Просмотреть файл

@ -521,4 +521,114 @@ describe("Modelerfour.Schemas", () => {
expect(schema).toBeInstanceOf(UriSchema);
});
});
describe("Validate auto-correct allOf/oneOf/anyOf", () => {
it("Auto-corrects schema with no type and allOf to type: object", async () => {
const spec = createTestSpec();
addSchema(
spec,
"Widget",
{
description: "A widget",
allOf: [{ $ref: "#/components/schemas/BaseWidget" }, { required: ["id"] }],
},
{ name: "Widget" },
);
addSchema(
spec,
"BaseWidget",
{
type: "object",
properties: {
id: { type: "string" },
color: { type: "string" },
},
},
{ name: "BaseWidget" },
);
const codeModel = await runModeler(spec);
const widget = findByName("Widget", codeModel.schemas.objects);
expect(widget).toBeDefined();
expect(widget?.type).toBe("object");
});
it("Auto-corrects schema with no type and allOf with no type to type: object", async () => {
const spec = createTestSpec();
addSchema(
spec,
"Widget",
{
description: "A widget",
allOf: [{ $ref: "#/components/schemas/BaseWidget" }, { $ref: "#/components/schemas/Metadata" }],
},
{ name: "Widget" },
);
addSchema(
spec,
"BaseWidget",
{
properties: {
id: { type: "string" },
color: { type: "string" },
},
},
{ name: "BaseWidget" },
);
addSchema(
spec,
"Metadata",
{
properties: {
createdAt: { type: "string", format: "date-time" },
updatedAt: { type: "string", format: "date-time" },
},
},
{ name: "Metadata" },
);
const codeModel = await runModeler(spec);
const widget = findByName("Widget", codeModel.schemas.objects);
expect(widget).toBeDefined();
expect(widget?.type).toBe("object");
});
it("Does not auto-correct schema with no type and oneOf with elements that are not type: object", async () => {
const spec = createTestSpec();
addSchema(
spec,
"Prompt",
{
description: "Prompt",
oneOf: [{ type: "string" }, { type: "array", items: { type: "string" } }],
},
{ name: "Prompt" },
);
addSchema(
spec,
"Request",
{
description: "Request",
properties: {
prompt: { $ref: "#/components/schemas/Prompt" },
},
},
{ name: "Request" },
);
const codeModel = await runModeler(spec);
const request = findByName("Request", codeModel.schemas.objects);
expect(request).toBeDefined();
expect(request?.properties?.find((x) => x.serializedName === "prompt")).toBeDefined();
const promptSchema = request?.properties?.find((x) => x.serializedName === "prompt")?.schema;
expect(promptSchema).toBeDefined();
expect(promptSchema?.type).toBe("any");
});
});
});

Просмотреть файл

@ -179,4 +179,72 @@ describe("Prechecker", () => {
});
});
});
describe("Validate auto-correct allOf/oneOf/anyOf", () => {
let spec: any;
beforeEach(() => {
spec = createTestSpec();
});
it("Auto-corrects schema with no type and allOf to type: object", async () => {
addSchema(
spec,
"Widget",
{
description: "A widget",
allOf: [{ $ref: "#/components/schemas/BaseWidget" }, { required: ["id"] }],
},
{ name: "Widget" },
);
addSchema(
spec,
"BaseWidget",
{
type: "object",
properties: {
id: { type: "string" },
color: { type: "string" },
},
},
{ name: "BaseWidget" },
);
const { session, errors, warnings } = await createTestSessionFromModel<Model>({}, spec);
const prechecker = new QualityPreChecker(session);
prechecker.process();
expect(errors).toHaveLength(0);
// Filter to get the warning about the auto-correct
const autoCorrectWarnings = warnings.filter((w) => w.Key[1] === "SchemaMissingType");
expect(autoCorrectWarnings).toHaveLength(1);
expect(autoCorrectWarnings[0]).toEqual({
Channel: "warning",
Details: undefined,
Key: ["PreCheck", "SchemaMissingType"],
Text: "The schema 'Widget' with an undefined type and 'allOf'/'anyOf'/'oneOf' is a bit ambiguous. This has been auto-corrected to 'type:object'",
Source: [{ Position: { path: ["components", "schemas", "Widget"] }, document: "openapi-3.json" }],
});
});
it("Does not auto-correct schema with no type and oneOf with elements that are not type: object", async () => {
addSchema(
spec,
"Prompt",
{
description: "Prompt",
oneOf: [{ type: "string" }, { type: "array", items: { type: "string" } }],
},
{ name: "Prompt" },
);
const { session, errors, warnings } = await createTestSessionFromModel<Model>({}, spec);
const prechecker = new QualityPreChecker(session);
prechecker.process();
expect(errors).toHaveLength(0);
// Filter to get the warning about the auto-correct
const autoCorrectWarnings = warnings.filter((w) => w.Key[1] === "SchemaMissingType");
expect(autoCorrectWarnings).toHaveLength(0);
});
});
});

Просмотреть файл

@ -13,6 +13,7 @@ export interface TestSessionInput {
export interface TestSession<T> {
session: Session<T>;
errors: Array<any>;
warnings: Array<any>;
}
async function readData(folder: string, ...files: Array<string>): Promise<Map<string, TestSessionInput>> {
@ -52,12 +53,16 @@ export async function createTestSession<TInputModel>(
): Promise<TestSession<TInputModel>> {
const models = Array.isArray(inputs) ? inputs.reduce((m, x) => m.set(x.filename, x), new Map()) : inputs;
const errors: Array<any> = [];
const warnings: Array<any> = [];
const sendMessage = (message: any): void => {
if (message.Channel === "warning" || message.Channel === "error" || message.Channel === "verbose") {
if (message.Channel === "error") {
errors.push(message);
}
if (message.Channel === "warning") {
warnings.push(message);
}
}
};
@ -73,5 +78,5 @@ export async function createTestSession<TInputModel>(
UpdateConfigurationFile: (filename: string, content: string) => {},
GetConfigurationFile: (filename: string) => Promise.resolve(""),
} as any);
return { session, errors };
return { session, errors, warnings };
}