Skip to content

Commit

Permalink
perf(spec-parser): update to support undefined schema type (#12424)
Browse files Browse the repository at this point in the history 
* perf(spec-parser): update to support undefined schema type

* perf: update test cases

* perf: remove unused comment

* perf: update test case

---------

Co-authored-by: rentu <rentu>
  • Loading branch information
@SLdragon
SLdragon authored Sep 20, 2024
1 parent 8c055b3 commit 066da54
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 23 deletions.
4 changes: 2 additions & 2 deletions packages/spec-parser/src/adaptiveCardGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class AdaptiveCardGenerator {
return [template];
}
// some schema may not contain type but contain properties
if (schema.type === "object" || (!schema.type && schema.properties)) {
if (Utils.isObjectSchema(schema)) {
const { properties } = schema;
const result: Array<TextBlockElement | ImageElement | ArrayElement> = [];
for (const property in properties) {
Expand Down Expand Up @@ -179,7 +179,7 @@ export class AdaptiveCardGenerator {

// Find the first array property in the response schema object with the well-known name
static getResponseJsonPathFromSchema(schema: OpenAPIV3.SchemaObject): string {
if (schema.type === "object" || (!schema.type && schema.properties)) {
if (Utils.isObjectSchema(schema)) {
const { properties } = schema;
for (const property in properties) {
const schema = properties[property] as OpenAPIV3.SchemaObject;
Expand Down
2 changes: 1 addition & 1 deletion packages/spec-parser/src/manifestUpdater.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class ManifestUpdater {
if (requestBody) {
const requestJsonBody = requestBody.content!["application/json"];
const requestBodySchema = requestJsonBody.schema as OpenAPIV3.SchemaObject;
if (requestBodySchema.type === "object") {
if (Utils.isObjectSchema(requestBodySchema)) {
for (const property in requestBodySchema.properties) {
const schema = requestBodySchema.properties[property] as OpenAPIV3.SchemaObject;
ManifestUpdater.checkSchema(schema, method, pathUrl);
Expand Down
10 changes: 7 additions & 3 deletions packages/spec-parser/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,21 @@ import { SpecParserError } from "./specParserError";

export class Utils {
static hasNestedObjectInSchema(schema: OpenAPIV3.SchemaObject): boolean {
if (schema.type === "object") {
if (this.isObjectSchema(schema)) {
for (const property in schema.properties) {
const nestedSchema = schema.properties[property] as OpenAPIV3.SchemaObject;
if (nestedSchema.type === "object") {
if (this.isObjectSchema(nestedSchema)) {
return true;
}
}
}
return false;
}

static isObjectSchema(schema: OpenAPIV3.SchemaObject): boolean {
return schema.type === "object" || (!schema.type && !!schema.properties);
}

static containMultipleMediaTypes(
bodyObject: OpenAPIV3.RequestBodyObject | OpenAPIV3.ResponseObject
): boolean {
Expand Down Expand Up @@ -311,7 +315,7 @@ export class Utils {
} else {
optionalParams.push(parameter);
}
} else if (schema.type === "object") {
} else if (Utils.isObjectSchema(schema)) {
const { properties } = schema;
for (const property in properties) {
let isRequired = false;
Expand Down
3 changes: 2 additions & 1 deletion packages/spec-parser/src/validators/copilotValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SpecValidationResult,
} from "../interfaces";
import { Validator } from "./validator";
import { Utils } from "../utils";

export class CopilotValidator extends Validator {
constructor(spec: OpenAPIV3.Document, options: ParseOptions) {
Expand Down Expand Up @@ -84,7 +85,7 @@ export class CopilotValidator extends Validator {
if (requestJsonBody) {
const requestBodySchema = requestJsonBody.schema as OpenAPIV3.SchemaObject;

if (requestBodySchema.type !== "object") {
if (!Utils.isObjectSchema(requestBodySchema)) {
result.reason.push(ErrorType.PostBodySchemaIsNotJson);
}

Expand Down
18 changes: 3 additions & 15 deletions packages/spec-parser/src/validators/validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ export abstract class Validator {
const isRequiredWithoutDefault = isRequired && schema.default === undefined;
const isCopilot = this.projectType === ProjectType.Copilot;

if (isCopilot && this.hasNestedObjectInSchema(schema)) {
if (isCopilot && Utils.hasNestedObjectInSchema(schema)) {
paramResult.isValid = false;
paramResult.reason = [ErrorType.RequestBodyContainsNestedObject];
return paramResult;
Expand All @@ -280,7 +280,7 @@ export abstract class Validator {
} else {
paramResult.optionalNum = paramResult.optionalNum + 1;
}
} else if (schema.type === "object") {
} else if (Utils.isObjectSchema(schema)) {
const { properties } = schema;
for (const property in properties) {
let isRequired = false;
Expand Down Expand Up @@ -323,7 +323,7 @@ export abstract class Validator {
const param = paramObject[i];
const schema = param.schema as OpenAPIV3.SchemaObject;

if (isCopilot && this.hasNestedObjectInSchema(schema)) {
if (isCopilot && Utils.hasNestedObjectInSchema(schema)) {
paramResult.isValid = false;
paramResult.reason.push(ErrorType.ParamsContainsNestedObject);
continue;
Expand Down Expand Up @@ -372,16 +372,4 @@ export abstract class Validator {

return paramResult;
}

private hasNestedObjectInSchema(schema: OpenAPIV3.SchemaObject): boolean {
if (schema.type === "object") {
for (const property in schema.properties) {
const nestedSchema = schema.properties[property] as OpenAPIV3.SchemaObject;
if (nestedSchema.type === "object") {
return true;
}
}
}
return false;
}
}
4 changes: 3 additions & 1 deletion packages/spec-parser/test/manifestUpdater.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5219,13 +5219,15 @@ describe("generateCommands", () => {
content: {
"application/json": {
schema: {
type: "object",
required: ["name"],
properties: {
name: {
type: "string",
description: "Name of the pet",
},
unknown: {
type: "unknown",
},
},
},
},
Expand Down
22 changes: 22 additions & 0 deletions packages/spec-parser/test/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,28 @@ describe("utils", () => {
});
});

describe("isObjectSchema", () => {
it('should return true when schema.type is "object"', () => {
const schema: OpenAPIV3.SchemaObject = { type: "object" };
expect(Utils.isObjectSchema(schema)).to.be.true;
});

it("should return true when schema.type is not defined but schema.properties is defined", () => {
const schema: OpenAPIV3.SchemaObject = { properties: { prop1: { type: "string" } } };
expect(Utils.isObjectSchema(schema)).to.be.true;
});

it("should return false when schema.type is not defined and schema.properties is not defined", () => {
const schema: OpenAPIV3.SchemaObject = {};
expect(Utils.isObjectSchema(schema)).to.be.false;
});

it('should return false when schema.type is defined but not "object"', () => {
const schema: OpenAPIV3.SchemaObject = { type: "string" };
expect(Utils.isObjectSchema(schema)).to.be.false;
});
});

describe("convertPathToCamelCase", () => {
it("should convert a path to camel case", () => {
const path = "this/is/a/{test}/path";
Expand Down
133 changes: 133 additions & 0 deletions packages/spec-parser/test/validator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2152,6 +2152,66 @@ describe("Validator", () => {
assert.deepEqual(reason, [ErrorType.PostBodySchemaIsNotJson]);
});

it("should return true if method is POST, and request body schema type is undefined but contains properties", () => {
const method = "POST";
const path = "/users";
const spec = {
servers: [
{
url: "https://example.com",
},
],
paths: {
"/users": {
post: {
requestBody: {
content: {
"application/json": {
schema: {
properties: {
name: {
type: "string",
},
},
},
},
},
},
responses: {
200: {
content: {
"application/json": {
schema: {
type: "object",
properties: {
name: {
type: "string",
},
},
},
},
},
},
},
},
},
},
};

const options: ParseOptions = {
allowMissingId: true,
allowAPIKeyAuth: false,
allowMultipleParameters: false,
allowOauth2: false,
projectType: ProjectType.Copilot,
allowMethods: ["get", "post"],
};

const validator = ValidatorFactory.create(spec as any, options);
const { isValid, reason } = validator.validateAPI(method, path);
assert.strictEqual(isValid, true);
});

it("should return true if there is no parameters for copilot", () => {
const method = "GET";
const path = "/users";
Expand Down Expand Up @@ -2612,6 +2672,79 @@ describe("Validator", () => {
assert.deepEqual(reason, [ErrorType.RequestBodyContainsNestedObject]);
});

it("should return false if method is POST, but requestBody contain nested object with undefined type", () => {
const method = "POST";
const path = "/users";
const spec = {
servers: [
{
url: "https://example.com",
},
],
paths: {
"/users": {
post: {
parameters: [
{
in: "query",
required: true,
schema: { type: "string" },
},
],
requestBody: {
content: {
"application/json": {
schema: {
required: ["name"],
properties: {
name: {
properties: {
id: {
type: "string",
},
},
},
},
},
},
},
},
responses: {
200: {
content: {
"application/json": {
schema: {
type: "object",
properties: {
name: {
type: "string",
},
},
},
},
},
},
},
},
},
},
};

const options: ParseOptions = {
allowMissingId: true,
allowAPIKeyAuth: false,
allowMultipleParameters: false,
allowOauth2: false,
projectType: ProjectType.Copilot,
allowMethods: ["get", "post"],
};

const validator = ValidatorFactory.create(spec as any, options);
const { isValid, reason } = validator.validateAPI(method, path);
assert.strictEqual(isValid, false);
assert.deepEqual(reason, [ErrorType.RequestBodyContainsNestedObject]);
});

it("should return false if contain circular reference", () => {
const method = "POST";
const path = "/users";
Expand Down

0 comments on commit 066da54

Please sign in to comment.