Refactor relative references (#404)

* Refactor resolve relative references and remove unecessary parsing

* Remove unused references from x-ms-examples

* Update changelog and package.json

ChangeLog.md

@@ -1,6 +1,11 @@
 # Changelog
 
-### 03/20/2019 0.14.6
+### 03/20/2019 0.14.8
+
+- Don't parse examples when not needed.
+- Remove example references when not needed to avoid "UNRESOLVABLE_REFERENCE" errors.
+
+### 03/20/2019 0.14.7
 
 - Change test runner to jest.
 

lib/util/utils.ts

@@ -62,10 +62,12 @@ export function generateRandomId(prefix: string, existingIds: {}): string {
 
 export interface Reference {
   readonly filePath?: string
-  readonly localReference?: {
-    readonly value: string
-    readonly accessorProperty: string
-  }
+  readonly localReference?: LocalReference
+}
+
+export interface LocalReference {
+  readonly value: string
+  readonly accessorProperty: string
 }
 
 /*

lib/validators/specResolver.ts

@@ -1,12 +1,12 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
-
 import { Suppression } from "@azure/openapi-markdown"
 import { isArray, map, toArray } from "@ts-common/iterator"
 import * as jsonParser from "@ts-common/json-parser"
 import * as ps from "@ts-common/property-set"
 import { arrayMap } from "@ts-common/source-map"
 import * as sm from "@ts-common/string-map"
+import * as jsonPointer from "json-pointer"
 import * as _ from "lodash"
 import * as path from "path"
 import {
@@ -27,7 +27,6 @@ import * as jsonUtils from "../util/jsonUtils"
 import { log } from "../util/logging"
 import { getOperations } from "../util/methods"
 import * as utils from "../util/utils"
-
 import { PolymorphicTree } from "./polymorphicTree"
 import { resolveNestedDefinitions } from "./resolveNestedDefinitions"
 
@@ -352,71 +351,110 @@ export class SpecResolver {
       docPath = utils.joinPath(docDir, parsedReference.filePath)
     }
 
-    const result = await jsonUtils.parseJson(suppression, docPath, this.reportError, this.docsCache)
-    if (!parsedReference.localReference) {
+    if (parsedReference.localReference) {
+      await this.resolveLocalReference(
+        slicedRefName,
+        doc,
+        docPath,
+        node,
+        parsedReference.localReference,
+        suppression
+      )
+    } else {
       // Since there is no local reference we will replace the key in the object with the parsed
       // json (relative) file it is referring to.
-      const regex = /.*x-ms-examples.*/gi
-      if (
-        this.options.shouldResolveXmsExamples ||
-        (!this.options.shouldResolveXmsExamples && slicedRefName.match(regex) === null)
-      ) {
-        // TODO: doc should have a type
-        // We set a function `() => result` instead of an object `result` to avoid
-        // reference resolution in the examples.
-        utils.setObject(doc as {}, slicedRefName, () => result)
-      }
-    } else {
-      // resolve the local reference.
-      // make the reference local to the doc being processed
-      node.$ref = parsedReference.localReference.value
-      // TODO: doc should have a type
-      utils.setObject(doc as {}, slicedRefName, node)
-      const slicedLocalReferenceValue = parsedReference.localReference.value.slice(1)
-      let referencedObj = this.visitedEntities[slicedLocalReferenceValue]
-      if (!referencedObj) {
-        // We get the definition/parameter from the relative file and then add it (make it local)
-        // to the doc (i.e. self.specInJson) being processed.
-        referencedObj = utils.getObject(result, slicedLocalReferenceValue) as SchemaObject
-        utils.setObject(this.specInJson, slicedLocalReferenceValue, referencedObj)
-        this.visitedEntities[slicedLocalReferenceValue] = referencedObj
-        await this.resolveRelativePaths(suppression, referencedObj, docPath, "all")
-        // After resolving a model definition, if there are models that have an allOf on that model
-        // definition.
-        // It may be possible that those models are not being referenced anywhere. Hence, we must
-        // ensure that they are consumed as well. Example model "CopyActivity" in file
-        // arm-datafactory/2017-03-01-preview/swagger/entityTypes/Pipeline.json is having an allOf
-        // on model "Activity". Spec "datafactory.json" has references to "Activity" in
-        // Pipeline.json but there are no references to "CopyActivity". The following code, ensures
-        // that we do not forget such models while resolving relative swaggers.
-        if (result && result.definitions) {
-          const definitions = result.definitions
-          const unresolvedDefinitions: Array<() => Promise<void>> = []
+      await this.resolveRemoteReference(slicedRefName, doc, docPath, suppression)
+    }
+  }
 
-          const processDefinition = ([defName, def]: sm.Entry<SchemaObject>) => {
-            unresolvedDefinitions.push(async () => {
-              const allOf = def.allOf
-              if (allOf) {
-                const matchFound = allOf.some(
-                  () => !this.visitedEntities[`/definitions/${defName}`]
-                )
-                if (matchFound) {
-                  const slicedDefinitionRef = `/definitions/${defName}`
-                  const definitionObj = definitions[defName]
-                  utils.setObject(this.specInJson, slicedDefinitionRef, definitionObj)
-                  this.visitedEntities[slicedDefinitionRef] = definitionObj
-                  await this.resolveRelativePaths(suppression, definitionObj, docPath, "all")
-                }
+  /**
+   * Resolves references local to the file.
+   */
+  private async resolveLocalReference(
+    slicedRefName: string,
+    doc: unknown,
+    docPath: string,
+    node: { $ref: string },
+    localReference: utils.LocalReference,
+    suppression: Suppression | undefined
+  ) {
+    // resolve the local reference.
+    // make the reference local to the doc being processed
+    const result = await jsonUtils.parseJson(suppression, docPath, this.reportError, this.docsCache)
+
+    node.$ref = localReference.value
+    // TODO: doc should have a type
+    utils.setObject(doc as {}, slicedRefName, node)
+    const slicedLocalReferenceValue = localReference.value.slice(1)
+    let referencedObj = this.visitedEntities[slicedLocalReferenceValue]
+    if (!referencedObj) {
+      // We get the definition/parameter from the relative file and then add it (make it local)
+      // to the doc (i.e. self.specInJson) being processed.
+      referencedObj = utils.getObject(result, slicedLocalReferenceValue) as SchemaObject
+      utils.setObject(this.specInJson, slicedLocalReferenceValue, referencedObj)
+      this.visitedEntities[slicedLocalReferenceValue] = referencedObj
+      await this.resolveRelativePaths(suppression, referencedObj, docPath, "all")
+      // After resolving a model definition, if there are models that have an allOf on that model
+      // definition.
+      // It may be possible that those models are not being referenced anywhere. Hence, we must
+      // ensure that they are consumed as well. Example model "CopyActivity" in file
+      // arm-datafactory/2017-03-01-preview/swagger/entityTypes/Pipeline.json is having an allOf
+      // on model "Activity". Spec "datafactory.json" has references to "Activity" in
+      // Pipeline.json but there are no references to "CopyActivity". The following code, ensures
+      // that we do not forget such models while resolving relative swaggers.
+      if (result && result.definitions) {
+        const definitions = result.definitions
+        const unresolvedDefinitions: Array<() => Promise<void>> = []
+
+        const processDefinition = ([defName, def]: sm.Entry<SchemaObject>) => {
+          unresolvedDefinitions.push(async () => {
+            const allOf = def.allOf
+            if (allOf) {
+              const matchFound = allOf.some(() => !this.visitedEntities[`/definitions/${defName}`])
+              if (matchFound) {
+                const slicedDefinitionRef = `/definitions/${defName}`
+                const definitionObj = definitions[defName]
+                utils.setObject(this.specInJson, slicedDefinitionRef, definitionObj)
+                this.visitedEntities[slicedDefinitionRef] = definitionObj
+                await this.resolveRelativePaths(suppression, definitionObj, docPath, "all")
               }
-            })
-          }
-
-          for (const entry of sm.entries(result.definitions)) {
-            processDefinition(entry)
-          }
-
-          await utils.executePromisesSequentially(unresolvedDefinitions)
+            }
+          })
         }
+
+        for (const entry of sm.entries(result.definitions)) {
+          processDefinition(entry)
+        }
+
+        await utils.executePromisesSequentially(unresolvedDefinitions)
+      }
+    }
+  }
+
+  /**
+   * Resolves remote references for the document
+   */
+  private async resolveRemoteReference(
+    slicedRefName: string,
+    doc: unknown,
+    docPath: string,
+    suppression: Suppression | undefined
+  ) {
+    const regex = /.*x-ms-examples.*/gi
+    if (this.options.shouldResolveXmsExamples || slicedRefName.match(regex) === null) {
+      const result = await jsonUtils.parseJson(
+        suppression,
+        docPath,
+        this.reportError,
+        this.docsCache
+      )
+
+      // We set a function `() => result` instead of an object `result` to avoid
+      // reference resolution in the examples.
+      utils.setObject(doc as {}, slicedRefName, () => result)
+    } else {
+      if (jsonPointer.has(doc as {}, slicedRefName)) {
+        jsonPointer.remove(doc as {}, slicedRefName)
       }
     }
   }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "oav",
-  "version": "0.14.7",
+  "version": "0.14.8",
   "author": {
     "name": "Microsoft Corporation",
     "email": "azsdkteam@microsoft.com",