Skip to content
This repository has been archived by the owner on Oct 11, 2023. It is now read-only.

Commit

Permalink
Put the removal of empty child schemas behind a flag (#377)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored Jan 4, 2021
1 parent 0dcfb6d commit cc1767b
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 14 deletions.
5 changes: 5 additions & 0 deletions modelerfour/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ modelerfour:
# remodeler to modelerfour.
always-seal-x-ms-enum: false|true

# In the case where a type only definition is to inherit another type remove it.
# e.g. ChildSchema: {allOf: [ParentSchema]}.
# In this case ChildSchema will be removed and all reference to it will be updated to point to ParentSchema
remove-empty-child-schemas: false|true

# customization of the identifier normalization and naming provided by the prenamer.
# pascal|pascalcase - MultiWordIdentifier
# camel|camelcase - multiWordIdentifier
Expand Down
7 changes: 7 additions & 0 deletions modelerfour/src/modeler/modelerfour-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ export interface ModelerFourOptions {
"prenamer"?: boolean;

"resolve-schema-name-collisons"?: boolean;

/**
* In the case where a type only definition is to inherit another type remove it.
* @example ChildSchema: {allOf: [ParentSchema]}.
* In this case ChildSchema will be removed and all reference to it will be updated to point to ParentSchema
*/
"remove-empty-child-schemas"?: boolean;
}

export interface ModelerFourNamingOptions {
Expand Down
14 changes: 7 additions & 7 deletions modelerfour/src/quality-precheck/prechecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ export class QualityPreChecker {
}

private removeDuplicateSchemas(name: string, schema1: DereferencedSchema, schema2: DereferencedSchema) {
const { keep: schemaToKeep, remove: schemaToRemove} = this.findSchemaToRemove(schema1, schema2);
const { keep: schemaToKeep, remove: schemaToRemove } = this.findSchemaToRemove(schema1, schema2);
delete this.input.components!.schemas![schemaToRemove.key];
const text = JSON.stringify(this.input);
this.input = JSON.parse(
Expand Down Expand Up @@ -346,7 +346,7 @@ export class QualityPreChecker {
);
}
}

this.session.verbose(
`Schema ${name} has multiple identical declarations, reducing to just one - removing: ${schemaToRemove.key}, keeping: ${schemaToKeep.key}`,
["PreCheck", "ReducingSchema"],
Expand Down Expand Up @@ -377,9 +377,9 @@ export class QualityPreChecker {
this.input = JSON.parse(
text.replace(new RegExp(`"\\#\\/components\\/schemas\\/${key}"`, "g"), `"${$ref}"`),
);
const location = schema["x-ms-metadata"].originalLocations[0].replace(/^.*\//, "");
const location = schema["x-ms-metadata"].originalLocations?.[0]?.replace(/^.*\//, "");
delete this.input.components?.schemas?.[key];
if (schema["x-internal-autorest-anonymous-schema"]) {
delete schemas[key];
this.session.warning(
`An anonymous inline schema for property '${location.replace(
/-/g,
Expand All @@ -388,8 +388,6 @@ export class QualityPreChecker {
["PreCheck", "AllOfWhenYouMeantRef"],
);
} else {
// NOTE: Disabled removing of non-anonymous schema for now until
// it has been discussed in Azure/autorest.modelerfour#278
this.session.warning(
`Schema '${location}' is using an 'allOf' instead of a $ref. This creates a wasteful anonymous type when generating code.`,
["PreCheck", "AllOfWhenYouMeantRef"],
Expand Down Expand Up @@ -510,7 +508,9 @@ export class QualityPreChecker {
}

process() {
this.fixUpSchemasThatUseAllOfInsteadOfJustRef();
if (this.options["remove-empty-child-schemas"]) {
this.fixUpSchemasThatUseAllOfInsteadOfJustRef();
}

this.fixUpObjectsWithoutType();

Expand Down
61 changes: 54 additions & 7 deletions modelerfour/test/quality-precheck/prechecker.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { addSchema, createTestSessionFromModel, createTestSpec } from "../utils";
import { QualityPreChecker } from "../../src/quality-precheck/prechecker";
import { Model, Refable, Dereferenced, dereference, Schema } from "@azure-tools/openapi";
import { ModelerFourOptions } from "modeler/modelerfour-options";

class PreCheckerClient {
private constructor(private input: Model, public result: Model) {}
Expand All @@ -9,8 +10,8 @@ class PreCheckerClient {
return dereference(this.input, item);
}

static async create(spec: Model): Promise<PreCheckerClient> {
const { session, errors } = await createTestSessionFromModel<Model>({}, spec);
static async create(spec: Model, config: ModelerFourOptions = {}): Promise<PreCheckerClient> {
const { session, errors } = await createTestSessionFromModel<Model>({ modelerfour: config }, spec);
const prechecker = await new QualityPreChecker(session).init();
expect(errors.length).toBe(0);

Expand Down Expand Up @@ -60,13 +61,13 @@ describe("Prechecker", () => {
const spec = createTestSpec();

addSchema(spec, "SiblingSchema", {
$ref: "#/components/schemas/MainSchema"
$ref: "#/components/schemas/MainSchema",
});

addSchema(spec, "MainSchema", {
type: "object",
"type": "object",
"x-ms-client-name": "MainSchema",
properties: {
"properties": {
name: {
type: "string",
},
Expand All @@ -78,9 +79,55 @@ describe("Prechecker", () => {
const schemas = model.components!.schemas!;
expect(schemas["SiblingSchema"]).toBeUndefined();
expect(schemas["MainSchema"]).not.toBeUndefined();

console.error(schemas["MainSchema"]);
const mainSchema: Schema = schemas["MainSchema"] as any;
expect(mainSchema.properties?.name.type).toEqual("string");
});

describe("Remove child types with no additional properties", () => {
let spec: any;

beforeEach(() => {
spec = createTestSpec();
addSchema(spec, "ChildSchema", {
allOf: [{ $ref: "#/components/schemas/ParentSchema" }],
});

addSchema(spec, "ParentSchema", {
type: "object",
properties: {
name: {
type: "string",
},
},
});
addSchema(spec, "Foo", {
type: "object",
properties: {
child: { $ref: "#/components/schemas/ChildSchema" },
},
});
});

it("Doesn't touch it by default", async () => {
const client = await PreCheckerClient.create(spec);
const schemas = client.result.components!.schemas!;
expect(schemas["ChildSchema"]).not.toBeUndefined();
expect(schemas["ParentSchema"]).not.toBeUndefined();
const foo = schemas["Foo"] as any;
expect(foo).not.toBeUndefined();
expect(foo.properties.child.$ref).toEqual("#/components/schemas/ChildSchema");
});

it("Remove the child type and points reference to it to its parent", async () => {
const client = await PreCheckerClient.create(spec, {
"remove-empty-child-schemas": true,
});
const schemas = client.result.components!.schemas!;
expect(schemas["ChildSchema"]).toBeUndefined();
expect(schemas["ParentSchema"]).not.toBeUndefined();
const foo = schemas["Foo"] as any;
expect(foo).not.toBeUndefined();
expect(foo.properties.child.$ref).toEqual("#/components/schemas/ParentSchema");
});
});
});

0 comments on commit cc1767b

Please sign in to comment.