Merge pull request #1620 from shockey/master

Always use resolved API definition in semantic validation

Author: shockey

src/plugins/validate-semantic/index.js

@@ -27,9 +27,8 @@ export default function SemanticValidatorsPlugin({getSystem}) {
       spec: {
         selectors: {
           jsonAsJS: createSelector(
-            state => state.get("resolvedSpec"),
-            state => state.get("json"),
-            (a,b) => (a || b).toJS()
+            state => state.get("resolved"),
+            (spec) => spec ? spec.toJS() : null
           ),
           definitions: createSelector(
             state => state.getIn(["json", "definitions"]),

src/plugins/validate-semantic/selectors.js

@@ -84,15 +84,15 @@ export const isResponseSchema = (state, node) => (sys) => {
 }
 
 export const allSchemas = () => (system) => {
-  const schemaSources = [
-    "allParameterSchemas",
-    "allResponseSchemas",
-    "allDefinitions",
-    "allHeaders",
-    "allSubSchemas",
-  ]
+  const { validateSelectors } = system
 
-  const selectors = schemaSources.map(name => system.validateSelectors[name]())
+  const selectors = [
+    validateSelectors.allParameterSchemas(),
+    validateSelectors.allResponseSchemas(),
+    validateSelectors.allDefinitions(),
+    validateSelectors.allHeaders(),
+    validateSelectors.allSubSchemas(),
+  ]
 
   return Promise.all(selectors)
     .then((schemasAr) => {

src/plugins/validate-semantic/validators/operations.js

@@ -6,15 +6,19 @@ export const validateOperationHasUniqueId = () => sys => {
       return nodes.reduce((acc, node) => {
         const value = node.node
 
-        if(value.operationId) {
-          if(seen.indexOf(value.operationId) > -1) {
+        // Swagger-Client messes with `value.operationId`, but puts the inital
+        // value in `__originalOperationId` when it resolves, so we use that.
+        const id = value.__originalOperationId
+
+        if(id) {
+          if(seen.indexOf(id) > -1) {
             acc.push({
               level: "error",
               message: "Operations must have unique operationIds.",
               path: [...node.path, "operationId"]
             })
           }
-          seen.push(value.operationId)
+          seen.push(id)
         }
         return acc
       }, [])

src/plugins/validate-semantic/validators/refs.js

@@ -1,13 +1,19 @@
+import get from "lodash/get"
+
 export const validateRefHasNoSiblings = () => system => {
-  return system.validateSelectors
-    .all$refs()
-    .then(nodes => {
+  return Promise.all([
+    system.validateSelectors.all$refArtifacts(),
+  ]).then(([nodes]) => {
+      const immSpecJson = system.specSelectors.specJson()
+      const specJson = immSpecJson.toJS ? immSpecJson.toJS() : {}
+
       return nodes.reduce((acc, node) => {
-        const parentValue = node.notRoot ? node.parent.node : null
-        const parentKeys = Object.keys(parentValue) || []
+        const unresolvedValue = get(specJson, node.parent.path) || {}
+        const unresolvedKeys = Object.keys(unresolvedValue) || []
 
-        parentKeys.forEach(k => {
-          if(k !== "$ref") {
+
+        unresolvedKeys.forEach(k => {
+          if(k !== "$ref" && unresolvedKeys.indexOf("$ref") > -1) {
             acc.push({
               message: `Sibling values are not allowed alongside $refs`,
               path: [...node.path.slice(0, -1), k],
@@ -26,7 +32,12 @@ export const validateUnusedDefinitions = () => (system) => {
     system.validateSelectors.all$refs(),
     system.validateSelectors.all$refArtifacts()
   ]).then(([refs, refArtifacts]) => {
-    const references = (refs || refArtifacts || []).map(node => node.node)
+    const references = (
+      (refs.length ? refs : null)
+      || (refArtifacts.length ? refArtifacts : null)
+      || []
+    ).map(node => node.node)
+
     const errors = []
 
     system.specSelectors.definitions()

test/plugins/validate/form-data.js renamed to test/plugins/validate-semantic/form-data.js

@@ -24,6 +24,28 @@ describe("validation plugin - semantic - paths", function(){
       return expectNoErrors(spec)
     })
 
+    it("should not return problems for a valid path-level definiton/declaration pair using a $ref", function(){
+      const spec = {
+        paths: {
+          "/CoolPath/{id}": {
+            parameters: [
+              { $ref: "#/parameters/id" }
+            ]
+          }
+        },
+        parameters: {
+          id: {
+            name: "id",
+            in: "path",
+            description: "An id",
+            required: true
+          }
+        }
+      }
+
+      return expectNoErrors(spec)
+    })
+
     it("should not return problems for a valid operation-level definiton/declaration pair", function(){
       const spec = {
         paths: {

test/plugins/validate/operations.js renamed to test/plugins/validate-semantic/operations.js

@@ -10,10 +10,13 @@ describe("validation plugin - semantic - refs", function() {
           paths: {
             "/CoolPath": {
               schema: {
-                $ref: "#/definition/abc",
+                $ref: "#/definitions/abc",
                 description: "My very cool schema"
               }
             }
+          },
+          definitions: {
+            abc: {}
           }
         }
 

test/plugins/validate/parameters.js renamed to test/plugins/validate-semantic/parameters.js

@@ -545,30 +545,34 @@ describe("validation plugin - semantic - schema", function() {
         // Given
         const spec = {
           paths: {
-            $ref: "#/definitions/asdf"
-          },
-          "definitions": {
-            "asdf": {
-              type: "object",
-              properties: {
-                slug: {
-                  type: "string",
-                  pattern: "^[-a-zA-Z0-9_]+\\Z"
+            "/": {
+              get: {
+                responses: {
+                  "200": {
+                    schema: {
+                      type: "object",
+                      properties: {
+                        slug: {
+                          type: "string",
+                          pattern: "^[-a-zA-Z0-9_]+\\Z"
+                        }
+                      }
+                    }
+                  }
                 }
               }
             }
-          }
+          },
         }
 
         // When
         return validateHelper(spec)
           .then(system => {
-
             // Then
             expect(system.errSelectors.allErrors().count()).toEqual(1)
             const firstError = system.errSelectors.allErrors().first().toJS()
             expect(firstError.message).toEqual(`"\\Z" anchors are not allowed in regular expression patterns`)
-            expect(firstError.path).toEqual(["definitions", "asdf", "properties", "slug", "pattern"])
+            expect(firstError.path).toEqual(["paths", "/", "get", "responses", "200", "schema", "properties", "slug", "pattern"])
           })
 
       })