Fix: Circular dependency issue with sibling schemas/definitions. (#375)

Azure · Dec 15, 2020 · 2736459 · 2736459
1 parent 8a8a88d
commit 2736459
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 43 deletions.
diff --git a/modelerfour/changelog.md b/modelerfour/changelog.md
@@ -5,6 +5,7 @@
   - **Feature** Added new flag `always-create-accept-parameter` to enable/disable accept param auto generation. ([PR 366](https://github.com/Azure/autorest.modelerfour/pull/366))
   - **Fix** Allow request with body being a file and `application/json` content-type. ([PR 363](https://github.com/Azure/autorest.modelerfour/pull/363))
   - **Fix** Dictionaries of dictionaries not being modeled as such(`dict[str, object]` instead of `dict[str, dict[str, str]]`). ([PR 372](https://github.com/Azure/autorest.modelerfour/pull/372))
+  - **Fix** Issue with sibling models(Model just being a ref of another) causing circular dependency exception. ([PR 375](https://github.com/Azure/autorest.modelerfour/pull/375))
   - **Fix** Issue with duplicates schemas names due to consequtive name duplicate removal. ([PR 374](https://github.com/Azure/autorest.modelerfour/pull/374))
 
 #### 4.15.x

diff --git a/modelerfour/src/quality-precheck/prechecker.ts b/modelerfour/src/quality-precheck/prechecker.ts
@@ -56,14 +56,18 @@ export async function processRequest(host: Host) {
   }
 }
 
+interface DereferencedSchema {
+  key: string;
+  value: Dereferenced<Schema>;
+}
+
 export class QualityPreChecker {
   input: oai3;
   options: ModelerFourOptions = {};
   protected interpret: Interpretations;
 
   constructor(protected session: Session<oai3>) {
     this.input = session.model; // shadow(session.model, filename);
-
     this.interpret = new Interpretations(session);
   }
 
@@ -244,48 +248,7 @@ export class QualityPreChecker {
               // found two schemas that are indeed the same.
               // stop, find all the $refs to the second one, and rewrite them to go to the first one.
               // then go back and start again.
-
-              delete this.input.components.schemas[schemas[1].key];
-              const text = JSON.stringify(this.input);
-              this.input = JSON.parse(
-                text.replace(
-                  new RegExp(`"\\#\\/components\\/schemas\\/${schemas[1].key}"`, "g"),
-                  `"#/components/schemas/${schemas[0].key}"`,
-                ),
-              );
-
-              // update metadata to match
-              if (this.input?.components?.schemas?.[schemas[0].key]) {
-                const primarySchema = this.resolve(this.input.components.schemas[schemas[0].key]);
-                const primaryMetadata = primarySchema.instance["x-ms-metadata"];
-                const secondaryMetadata = schemas[1].value.instance["x-ms-metadata"];
-
-                if (primaryMetadata && secondaryMetadata) {
-                  primaryMetadata.apiVersions = [
-                    ...new Set<string>([
-                      ...(primaryMetadata.apiVersions || []),
-                      ...(secondaryMetadata.apiVersions || []),
-                    ]),
-                  ];
-                  primaryMetadata.filename = [
-                    ...new Set<string>([...(primaryMetadata.filename || []), ...(secondaryMetadata.filename || [])]),
-                  ];
-                  primaryMetadata.originalLocations = [
-                    ...new Set<string>([
-                      ...(primaryMetadata.originalLocations || []),
-                      ...(secondaryMetadata.originalLocations || []),
-                    ]),
-                  ];
-                  primaryMetadata["x-ms-secondary-file"] = !(
-                    !primaryMetadata["x-ms-secondary-file"] || !secondaryMetadata["x-ms-secondary-file"]
-                  );
-                }
-              }
-              this.session.verbose(
-                `Schema ${name} has multiple identical declarations, reducing to just one - removing ${schemas[1].key} `,
-                ["PreCheck", "ReducingSchema"],
-              );
-
+              this.removeDuplicateSchemas(name, schemas[0], schemas[1]);
               // Restart the scan now that the duplicate has been removed
               return true;
             }
@@ -336,6 +299,60 @@ export class QualityPreChecker {
     return undefined;
   }
 
+  private findSchemaToRemove(
+    schema1: DereferencedSchema,
+    schema2: DereferencedSchema,
+  ): { keep: DereferencedSchema; remove: DereferencedSchema } {
+    const schema1Ref = this.input.components?.schemas?.[schema1.key].$ref;
+    // If schema1 is pointing to schema2 then we should delete schema1
+    if (schema1Ref && schema1Ref === `#/components/schemas/${schema2.key}`) {
+      return { remove: schema1, keep: schema2 };
+    }
+    return { remove: schema2, keep: schema1 };
+  }
+
+  private removeDuplicateSchemas(name: string, schema1: DereferencedSchema, schema2: DereferencedSchema) {
+    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(
+      text.replace(
+        new RegExp(`"\\#\\/components\\/schemas\\/${schemaToRemove.key}"`, "g"),
+        `"#/components/schemas/${schemaToKeep.key}"`,
+      ),
+    );
+
+    // update metadata to match
+    if (this.input?.components?.schemas?.[schemaToKeep.key]) {
+      const primarySchema = this.resolve(this.input.components.schemas[schemaToKeep.key]);
+      const primaryMetadata = primarySchema.instance["x-ms-metadata"];
+      const secondaryMetadata = schemaToRemove.value.instance["x-ms-metadata"];
+
+      if (primaryMetadata && secondaryMetadata) {
+        primaryMetadata.apiVersions = [
+          ...new Set<string>([...(primaryMetadata.apiVersions || []), ...(secondaryMetadata.apiVersions || [])]),
+        ];
+        primaryMetadata.filename = [
+          ...new Set<string>([...(primaryMetadata.filename || []), ...(secondaryMetadata.filename || [])]),
+        ];
+        primaryMetadata.originalLocations = [
+          ...new Set<string>([
+            ...(primaryMetadata.originalLocations || []),
+            ...(secondaryMetadata.originalLocations || []),
+          ]),
+        ];
+        primaryMetadata["x-ms-secondary-file"] = !(
+          !primaryMetadata["x-ms-secondary-file"] || !secondaryMetadata["x-ms-secondary-file"]
+        );
+      }
+    }
+
+    this.session.verbose(
+      `Schema ${name} has multiple identical declarations, reducing to just one - removing: ${schemaToRemove.key}, keeping: ${schemaToKeep.key}`,
+      ["PreCheck", "ReducingSchema"],
+    );
+  }
+
   fixUpSchemasThatUseAllOfInsteadOfJustRef() {
     const schemas = this.input.components?.schemas;
     if (schemas) {

diff --git a/modelerfour/test/quality-precheck/prechecker.test.ts b/modelerfour/test/quality-precheck/prechecker.test.ts
@@ -55,4 +55,32 @@ describe("Prechecker", () => {
       fail("No 'ChildSchema' found!");
     }
   });
+
+  it("remove the sibling schema with the $ref", async () => {
+    const spec = createTestSpec();
+
+    addSchema(spec, "SiblingSchema", {
+      $ref: "#/components/schemas/MainSchema"
+    });
+
+    addSchema(spec, "MainSchema", {
+      type: "object",
+      "x-ms-client-name": "MainSchema",
+      properties: {
+        name: {
+          type: "string",
+        },
+      },
+    });
+
+    const client = await PreCheckerClient.create(spec);
+    const model = client.result;
+    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");
+  });
 });