Merge pull request #396 from postmanlabs/feature/fix-schema-caching

Fixed issue where invalid cached schemas were being used for resolution.

lib/deref.js

@@ -111,7 +111,12 @@ module.exports = {
    * @param {*} schema (openapi) to resolve references.
    * @param {string} parameterSourceOption tells that the schema object is of request or response
    * @param {*} components components in openapi spec.
-   * @param {object} schemaResolutionCache stores already resolved references
+   * @param {object} schemaResolutionCache stores already resolved references - more structure detail below
+   * {'schema reference key': {
+   *    maxStack {Integer} : Defined as how deep of nesting level we reached while resolving schema that's being cached
+   *    resLevel {Integer} : Defined as nesting level at which schema that's being cached was resolved
+   *    schema {Object} : resolved schema that will be cached
+   * }}
    * @param {*} resolveFor - resolve refs for validation/conversion (value to be one of VALIDATION/CONVERSION)
    * @param {string} resolveTo The desired JSON-generation mechanism (schema: prefer using the JSONschema to
     generate a fake object, example: use specified examples as-is). Default: schema
@@ -133,6 +138,14 @@ module.exports = {
       return { value: ERR_TOO_MANY_LEVELS };
     }
 
+    // Update max stack reached for all current refs that's being resolved
+    if (!_.isEmpty(this._currentRefStack)) {
+      _.forEach(this._currentRefStack, (refKey) => {
+        _.set(schemaResolutionCache, [refKey, 'maxStack'],
+          Math.max(_.get(schemaResolutionCache, [refKey, 'maxStack'], 0), stack));
+      });
+    }
+
     if (!schema) {
       return { value: '<Error: Schema not found>' };
     }
@@ -167,8 +180,20 @@ module.exports = {
         // not throwing an error. We didn't find the reference - generate a dummy value
         return { value: 'reference ' + schema.$ref + ' not found in the OpenAPI spec' };
       }
-      if (schemaResolutionCache[refKey]) {
-        return schemaResolutionCache[refKey];
+
+      if (_.get(schemaResolutionCache, [refKey, 'schema'])) {
+        // maxStack for cached schema is how deep of nesting level we reached while resolving that schema
+        let maxStack = _.get(schemaResolutionCache, [refKey, 'maxStack'], 0),
+          // resLevel of perticuler cached schema is nesting level at which schema was resolved
+          resLevel = _.get(schemaResolutionCache, [refKey, 'resLevel'], stackLimit);
+
+        /**
+         * use cached schema if it was resolved at level lower or equal then at current stack level or
+         * if cached schema is resolved fully (it does not contain ERR_TOO_MANY_LEVELS value in sub schema)
+         */
+        if (resLevel <= stack || maxStack < stackLimit) {
+          return schemaResolutionCache[refKey].schema;
+        }
       }
       // something like #/components/schemas/PaginationEnvelope/properties/page
       // will be resolved - we don't care about anything after the components part
@@ -188,11 +213,19 @@ module.exports = {
       resolvedSchema = this._getEscaped(components, splitRef);
 
       if (resolvedSchema) {
+        // add current ref that's being resolved in ref stack
+        !_.isArray(this._currentRefStack) && (this._currentRefStack = []);
+        this._currentRefStack.push(refKey);
+
         let refResolvedSchema = this.resolveRefs(resolvedSchema, parameterSourceOption,
           components, schemaResolutionCache, resolveFor, resolveTo, stack, _.cloneDeep(seenRef));
 
+        // remove current ref that's being resolved from stack as soon as resolved
+        _.isArray(this._currentRefStack) && (this._currentRefStack.pop());
+
         if (refResolvedSchema && refResolvedSchema.value !== ERR_TOO_MANY_LEVELS) {
-          schemaResolutionCache[refKey] = refResolvedSchema;
+          _.set(schemaResolutionCache, [refKey, 'resLevel'], stack);
+          _.set(schemaResolutionCache, [refKey, 'schema'], refResolvedSchema);
         }
 
         return refResolvedSchema;

test/unit/deref.test.js

@@ -160,9 +160,7 @@ describe('DEREF FUNCTION TESTS ', function() {
         parameterSource = 'REQUEST',
         schemaResolutionCache = {},
         resolvedSchema = deref.resolveRefs(schema, parameterSource, componentsAndPaths, schemaResolutionCache);
-      expect(schemaResolutionCache).to.deep.equal({
-        '#/components/schema/request': resolvedSchema
-      });
+      expect(_.get(schemaResolutionCache, ['#/components/schema/request', 'schema'])).to.deep.equal(resolvedSchema);
       expect(resolvedSchema).to.deep.equal({
         type: 'object',
         properties: {
@@ -233,6 +231,87 @@ describe('DEREF FUNCTION TESTS ', function() {
       expect(output.pattern).to.eql(schema.pattern);
       done();
     });
+
+    it('should correctly resolve schema from schemaResoltionCache based on schema resolution level', function (done) {
+      let schema = {
+          $ref: '#/components/schemas/schemaUsed'
+        },
+        consumerSchema = {
+          type: 'object',
+          properties: { level2: {
+            type: 'object',
+            properties: { level3: {
+              type: 'object',
+              properties: { level4: {
+                type: 'object',
+                properties: { level5: {
+                  type: 'object',
+                  properties: { level6: {
+                    type: 'object',
+                    properties: { level7: {
+                      type: 'object',
+                      properties: { level8: {
+                        type: 'object',
+                        properties: { level9: { $ref: '#/components/schemas/schemaUsed' } }
+                      } }
+                    } }
+                  } }
+                } }
+              } }
+            } }
+          } }
+        },
+        componentsAndPaths = {
+          components: {
+            schemas: {
+              schemaUsed: {
+                'type': 'object',
+                'required': [
+                  'id',
+                  'name'
+                ],
+                'properties': {
+                  'id': {
+                    'type': 'integer',
+                    'format': 'int64'
+                  },
+                  'name': {
+                    'type': 'string'
+                  },
+                  'tag': {
+                    'type': 'string'
+                  }
+                }
+              }
+            }
+          }
+        },
+        parameterSource = 'REQUEST',
+        schemaResoltionCache = {},
+        resolvedConsumerSchema,
+        resolvedSchema;
+
+      resolvedConsumerSchema = deref.resolveRefs(consumerSchema, parameterSource, componentsAndPaths,
+        schemaResoltionCache);
+
+      // Consumer schema contains schema at nesting level 9, which results in impartial resolution of schema
+      expect(_.get(schemaResoltionCache, ['#/components/schemas/schemaUsed', 'resLevel'])).to.eql(9);
+      expect(_.get(resolvedConsumerSchema, _.join(_.map(_.range(1, 10), (ele) => {
+        return `properties.level${ele}`;
+      }), '.'))).to.not.deep.equal(componentsAndPaths.components.schemas.schemaUsed);
+      expect(_.get(schemaResoltionCache, ['#/components/schemas/schemaUsed', 'schema'])).to.not.deep
+        .equal(componentsAndPaths.components.schemas.schemaUsed);
+      resolvedSchema = deref.resolveRefs(schema, parameterSource, componentsAndPaths, schemaResoltionCache);
+
+      /**
+       * Even though schema cache contains schemaUsed as impartially cached,resolution were it's used again will
+       * depend on ongoing resolution level and schema is cached again if it's updated.
+       */
+      expect(resolvedSchema).to.deep.equal(componentsAndPaths.components.schemas.schemaUsed);
+      expect(_.get(schemaResoltionCache, ['#/components/schemas/schemaUsed', 'schema'])).to.deep
+        .equal(componentsAndPaths.components.schemas.schemaUsed);
+      done();
+    });
   });
 
   describe('resolveAllOf Function', function () {