Merge pull request #10 from hey-api/fix/improve-ref-pointer-inventory-check

fix: improve inventory check for pointers and add tests for multiple references

Author: carson2222

lib/__tests__/bundle.test.ts

@@ -1,14 +1,48 @@
-import path from 'node:path';
+import path from "path";
 
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it } from "vitest";
 
-import { $RefParser } from '..';
+import { $RefParser } from "..";
 
-describe('bundle', () => {
-  it('handles circular reference with description', async () => {
+describe("bundle", () => {
+  it("handles circular reference with description", async () => {
     const refParser = new $RefParser();
-    const pathOrUrlOrSchema = path.resolve('lib', '__tests__', 'spec', 'circular-ref-with-description.json');
+    const pathOrUrlOrSchema = path.resolve("lib", "__tests__", "spec", "circular-ref-with-description.json");
     const schema = await refParser.bundle({ pathOrUrlOrSchema });
     expect(schema).not.toBeUndefined();
   });
+
+  it("bundles multiple references to the same file correctly", async () => {
+    const refParser = new $RefParser();
+    const pathOrUrlOrSchema = path.resolve("lib", "__tests__", "spec", "multiple-refs.json");
+    const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
+
+    // First reference should be fully resolved (no $ref)
+    expect(schema.paths["/test1/{pathId}"].get.parameters[0].name).toBe("pathId");
+    expect(schema.paths["/test1/{pathId}"].get.parameters[0].schema.type).toBe("string");
+    expect(schema.paths["/test1/{pathId}"].get.parameters[0].schema.format).toBe("uuid");
+    expect(schema.paths["/test1/{pathId}"].get.parameters[0].$ref).toBeUndefined();
+
+    // Second reference should be remapped to point to the first reference
+    expect(schema.paths["/test2/{pathId}"].get.parameters[0].$ref).toBe(
+      "#/paths/~1test1~1%7BpathId%7D/get/parameters/0",
+    );
+
+    // Both should effectively resolve to the same data
+    const firstParam = schema.paths["/test1/{pathId}"].get.parameters[0];
+    const secondParam = schema.paths["/test2/{pathId}"].get.parameters[0];
+
+    // The second parameter should resolve to the same data as the first
+    expect(secondParam.$ref).toBeDefined();
+    expect(firstParam).toEqual({
+      name: "pathId",
+      in: "path",
+      required: true,
+      schema: {
+        type: "string",
+        format: "uuid",
+        description: "Unique identifier for the path",
+      },
+    });
+  });
 });

lib/__tests__/index.test.ts

@@ -1,43 +1,45 @@
-import path from 'node:path';
+import path from "node:path";
 
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it } from "vitest";
 
-import { getResolvedInput } from '../index';
+import { getResolvedInput } from "../index";
 
-describe('getResolvedInput', () => {
-  it('handles url', async () => {
-    const pathOrUrlOrSchema =  'https://foo.com';
+describe("getResolvedInput", () => {
+  it("handles url", async () => {
+    const pathOrUrlOrSchema = "https://foo.com";
     const resolvedInput = await getResolvedInput({ pathOrUrlOrSchema });
-    expect(resolvedInput.type).toBe('url');
+    expect(resolvedInput.type).toBe("url");
     expect(resolvedInput.schema).toBeUndefined();
-    expect(resolvedInput.path).toBe('https://foo.com/');
+    expect(resolvedInput.path).toBe("https://foo.com/");
   });
 
-  it('handles file', async () => {
-    const pathOrUrlOrSchema =  './path/to/openapi.json';
+  it("handles file", async () => {
+    const pathOrUrlOrSchema = "./path/to/openapi.json";
     const resolvedInput = await getResolvedInput({ pathOrUrlOrSchema });
-    expect(resolvedInput.type).toBe('file');
+    expect(resolvedInput.type).toBe("file");
     expect(resolvedInput.schema).toBeUndefined();
-    expect(resolvedInput.path).toBe(path.resolve('./path/to/openapi.json'));
+    expect(path.normalize(resolvedInput.path).toLowerCase()).toBe(
+      path.normalize(path.resolve("./path/to/openapi.json")).toLowerCase(),
+    );
   });
 
-  it('handles raw spec', async () => {
-    const pathOrUrlOrSchema =  {
+  it("handles raw spec", async () => {
+    const pathOrUrlOrSchema = {
       info: {
-        version: '1.0.0',
+        version: "1.0.0",
       },
-      openapi: '3.1.0',
+      openapi: "3.1.0",
       paths: {},
     };
     const resolvedInput = await getResolvedInput({ pathOrUrlOrSchema });
-    expect(resolvedInput.type).toBe('json');
+    expect(resolvedInput.type).toBe("json");
     expect(resolvedInput.schema).toEqual({
       info: {
-        version: '1.0.0',
+        version: "1.0.0",
       },
-      openapi: '3.1.0',
+      openapi: "3.1.0",
       paths: {},
     });
-    expect(resolvedInput.path).toBe('');
+    expect(resolvedInput.path).toBe("");
   });
 });

lib/__tests__/spec/multiple-refs.json

@@ -0,0 +1,34 @@
+{
+  "paths": {
+    "/test1/{pathId}": {
+      "get": {
+        "summary": "First endpoint using the same pathId schema",
+        "parameters": [
+          {
+            "$ref": "path-parameter.json#/pathId"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Test 1 response"
+          }
+        }
+      }
+    },
+    "/test2/{pathId}": {
+      "get": {
+        "summary": "Second endpoint using the same pathId schema",
+        "parameters": [
+          {
+            "$ref": "path-parameter.json#/pathId"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Test 2 response"
+          }
+        }
+      }
+    }
+  }
+}

lib/__tests__/spec/path-parameter.json

@@ -0,0 +1,12 @@
+{
+  "pathId": {
+    "name": "pathId",
+    "in": "path",
+    "required": true,
+    "schema": {
+      "type": "string",
+      "format": "uuid",
+      "description": "Unique identifier for the path"
+    }
+  }
+}

lib/bundle.ts

@@ -94,9 +94,10 @@ const inventory$Ref = <S extends object = JSONSchema>({
   const extended = $Ref.isExtended$Ref($ref);
   indirections += pointer.indirections;
 
+  // Check if this exact location (parent + key + pathFromRoot) has already been inventoried
   const existingEntry = findInInventory(inventory, $refParent, $refKey);
-  if (existingEntry) {
-    // This $Ref has already been inventoried, so we don't need to process it again
+  if (existingEntry && existingEntry.pathFromRoot === pathFromRoot) {
+    // This exact location has already been inventoried, so we don't need to process it again
     if (depth < existingEntry.depth || indirections < existingEntry.indirections) {
       removeFromInventory(inventory, existingEntry);
     } else {
@@ -172,7 +173,7 @@ const crawl = <S extends object = JSONSchema>({
   pathFromRoot: string;
 }) => {
   const obj = key === null ? parent : parent[key as keyof typeof parent];
-  
+
   if (obj && typeof obj === "object" && !ArrayBuffer.isView(obj)) {
     if ($Ref.isAllowed$Ref(obj)) {
       inventory$Ref({
@@ -359,17 +360,13 @@ function removeFromInventory(inventory: InventoryEntry[], entry: any) {
  * @param parser
  * @param options
  */
-export const bundle = (
-  parser: $RefParser,
-  options: ParserOptions,
-) => {
+export const bundle = (parser: $RefParser, options: ParserOptions) => {
   // console.log('Bundling $ref pointers in %s', parser.$refs._root$Ref.path);
-
   // Build an inventory of all $ref pointers in the JSON Schema
   const inventory: InventoryEntry[] = [];
   crawl<JSONSchema>({
     parent: parser,
-    key: 'schema',
+    key: "schema",
     path: parser.$refs._root$Ref.path + "#",
     pathFromRoot: "#",
     indirections: 0,