fix(core): dedupe paths containing special characters correctly

stoplightio · Jul 7, 2023 · 758de21 · 758de21
1 parent a79d26a
commit 758de21
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 6 deletions.
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -35,7 +35,7 @@
   },
   "dependencies": {
     "@stoplight/better-ajv-errors": "1.0.3",
-    "@stoplight/json": "~3.20.1",
+    "@stoplight/json": "~3.21.0",
     "@stoplight/path": "1.3.2",
     "@stoplight/spectral-parsers": "^1.0.0",
     "@stoplight/spectral-ref-resolver": "^1.0.0",

diff --git a/packages/core/src/__tests__/__fixtures__/gh-2500/input.json b/packages/core/src/__tests__/__fixtures__/gh-2500/input.json
@@ -0,0 +1,96 @@
+{
+  "openapi": "3.0.2",
+  "info": {
+    "title": "Service for GoalDetail",
+    "description": "Test You can find your company's API server",
+    "version": "1.0.0"
+  },
+  "servers": [
+    {
+      "url": "https://localhost/service-root"
+    }
+  ],
+  "paths": {
+    "/JobRelationship": {
+      "get": {
+        "summary": "Get entities from JobRelationship",
+        "responses": {
+          "200": {
+            "description": "Retrieved entities",
+            "content": {}
+          },
+          "4XX": {
+            "$ref": "#/components/responses/error"
+          }
+        }
+      }
+    },
+    "/JobRelationship({id})": {
+      "get": {
+        "summary": "Get entity from JobRelationship by key",
+        "responses": {
+          "200": {
+            "description": "Retrieved entities",
+            "content": {}
+          },
+          "4XX": {
+            "$ref": "#/components/responses/error"
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "error": {
+        "type": "object",
+        "required": ["error"],
+        "properties": {
+          "error": {
+            "type": "object",
+            "required": ["message"],
+            "properties": {
+              "message": {
+                "type": "string"
+              },
+              "target": {
+                "type": "string"
+              },
+              "details": {
+                "type": "array",
+                "items": {
+                  "type": "object",
+                  "required": ["message"],
+                  "properties": {
+                    "message": {
+                      "type": "string"
+                    },
+                    "target": {
+                      "type": "string"
+                    }
+                  }
+                }
+              },
+              "innererror": {
+                "type": "object",
+                "description": "The structure of this object is service-specific"
+              }
+            }
+          }
+        }
+      }
+    },
+    "responses": {
+      "error": {
+        "description": "Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/error"
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/packages/core/src/__tests__/__fixtures__/gh-2500/ruleset.ts b/packages/core/src/__tests__/__fixtures__/gh-2500/ruleset.ts
@@ -0,0 +1,18 @@
+import { Ruleset } from '../../../ruleset';
+import { DiagnosticSeverity } from '@stoplight/types';
+import { defined } from '@stoplight/spectral-functions';
+
+export default new Ruleset({
+  rules: {
+    'error-code-defined': {
+      message: '`code` property is missing in the Error object definition',
+      severity: DiagnosticSeverity.Error,
+      given:
+        "$.paths.*.*.responses[?(@property.match(/^(4|5)/))].content.'application/json'.schema.properties.error.properties",
+      then: {
+        field: 'code',
+        function: defined,
+      },
+    },
+  },
+});
diff --git a/packages/core/src/__tests__/spectral.jest.test.ts b/packages/core/src/__tests__/spectral.jest.test.ts
@@ -269,4 +269,31 @@ describe('Spectral', () => {
       }),
     ]);
   });
+
+  test('should dedupe paths containing special characters', async () => {
+    const s = new Spectral({ resolver: httpAndFileResolver });
+    const documentUri = path.join(__dirname, './__fixtures__/gh-2500/input.json');
+
+    s.setRuleset((await import('./__fixtures__/gh-2500/ruleset')).default);
+
+    const results = await s.run(new Document(fs.readFileSync(documentUri, 'utf8'), Parsers.Yaml, documentUri));
+
+    expect(results).toEqual([
+      expect.objectContaining({
+        code: 'error-code-defined',
+        path: ['components', 'schemas', 'error', 'properties', 'error', 'properties'],
+        source: documentUri,
+        range: {
+          end: {
+            character: 81,
+            line: 75,
+          },
+          start: {
+            character: 25,
+            line: 51,
+          },
+        },
+      }),
+    ]);
+  });
 });
diff --git a/packages/core/src/documentInventory.ts b/packages/core/src/documentInventory.ts
@@ -1,4 +1,4 @@
-import { decodePointerFragment, encodePointerFragment, extractSourceFromRef, isLocalRef } from '@stoplight/json';
+import { decodePointerFragment, encodePointerUriFragment, extractSourceFromRef, isLocalRef } from '@stoplight/json';
 import { extname, resolve } from '@stoplight/path';
 import { Dictionary, IParserResult, JsonPath } from '@stoplight/types';
 import { isObjectLike } from 'lodash';
@@ -113,7 +113,7 @@ export class DocumentInventory implements IDocumentInventory {
       let resolvedDoc = this.document;
 
       // Add '#' on the beginning of "path" to simplify the logic below.
-      const adjustedPath: string[] = ['#', ...path.map(String)];
+      const adjustedPath: string[] = ['#', ...path.map(encodePointerUriFragment).map(String)];
 
       // Walk through the segments of 'path' one at a time, looking for
       // json path locations containing a $ref.
@@ -123,7 +123,7 @@ export class DocumentInventory implements IDocumentInventory {
           refMapKey += '/';
         }
 
-        refMapKey += encodePointerFragment(segment);
+        refMapKey += segment;
 
         // If our current refMapKey value is in fact a key in refMap,
         // then we'll "reverse-resolve" it by replacing refMapKey with

diff --git a/yarn.lock b/yarn.lock
@@ -2606,7 +2606,21 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@stoplight/json@npm:^3.17.0, @stoplight/json@npm:^3.17.1, @stoplight/json@npm:^3.20.1, @stoplight/json@npm:~3.20.1":
+"@stoplight/json@npm:^3.17.0, @stoplight/json@npm:^3.17.1, @stoplight/json@npm:^3.20.1, @stoplight/json@npm:~3.21.0":
+  version: 3.21.0
+  resolution: "@stoplight/json@npm:3.21.0"
+  dependencies:
+    "@stoplight/ordered-object-literal": ^1.0.3
+    "@stoplight/path": ^1.3.2
+    "@stoplight/types": ^13.6.0
+    jsonc-parser: ~2.2.1
+    lodash: ^4.17.21
+    safe-stable-stringify: ^1.1
+  checksum: 16fe56a6804cd47837bd82d85a8500c4226669558f3feda55d8fb0cd615ca2261622963700f04f049cf30a3a9764eb3c861516003d948743b6ae85dbbabf8a59
+  languageName: node
+  linkType: hard
+
+"@stoplight/json@npm:~3.20.1":
   version: 3.20.1
   resolution: "@stoplight/json@npm:3.20.1"
   dependencies:
@@ -2674,7 +2688,7 @@ __metadata:
   resolution: "@stoplight/spectral-core@workspace:packages/core"
   dependencies:
     "@stoplight/better-ajv-errors": 1.0.3
-    "@stoplight/json": ~3.20.1
+    "@stoplight/json": ~3.21.0
     "@stoplight/path": 1.3.2
     "@stoplight/spectral-formats": "*"
     "@stoplight/spectral-functions": "*"