Merge pull request #659 from postmanlabs/feature/fix-validate-schema-example

Fix for conversion taking a long time in case of schema with example

Author: umeshp7

OPTIONS.md

@@ -8,7 +8,7 @@ requestParametersResolution|enum|Example, Schema|Schema|Select whether to genera
 exampleParametersResolution|enum|Example, Schema|Example|Select whether to generate the response parameters based on the [schema](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#schemaObject) or the [example](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#exampleObject) in the schema.|CONVERSION
 folderStrategy|enum|Paths, Tags|Paths|Select whether to create folders according to the spec’s paths or tags.|CONVERSION
 schemaFaker|boolean|-|true|Whether or not schemas should be faked.|CONVERSION
-stackLimit|integer|-|8|Number of nesting limit till which schema resolution will happen. Increasing this limit may result in more time to convert collection depending on complexity of specification. (To make sure this option works correctly "optimizeConversion" option needs to be disabled)|CONVERSION
+stackLimit|integer|-|10|Number of nesting limit till which schema resolution will happen. Increasing this limit may result in more time to convert collection depending on complexity of specification. (To make sure this option works correctly "optimizeConversion" option needs to be disabled)|CONVERSION
 includeAuthInfoInExample|boolean|-|true|Select whether to include authentication parameters in the example request|CONVERSION
 shortValidationErrors|boolean|-|false|Whether detailed error messages are required for request <> schema validation operations.|VALIDATION
 validationPropertiesToIgnore|array|-|[]|Specific properties (parts of a request/response pair) to ignore during validation. Must be sent as an array of strings. Valid inputs in the array: PATHVARIABLE, QUERYPARAM, HEADER, BODY, RESPONSE_HEADER, RESPONSE_BODY|VALIDATION

assets/json-schema-faker.js

@@ -15,7 +15,8 @@ var _ = require('lodash'),
   {
     handleExclusiveMaximum,
     handleExclusiveMinimum
-  } = require('./../lib/common/schemaUtilsCommon');
+  } = require('./../lib/common/schemaUtilsCommon'),
+  hash = require('object-hash');
 
 (function (global, factory) {
   typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() :
@@ -24100,7 +24101,7 @@ function extend() {
   var nullType = nullGenerator;
 
   // TODO provide types
-  function unique(path, items, value, sample, resolve, traverseCallback) {
+  function unique(path, items, value, sample, resolve, traverseCallback, seenSchemaCache) {
       var tmp = [], seen = [];
       function walk(obj) {
           var json = JSON.stringify(obj);
@@ -24113,15 +24114,15 @@ function extend() {
       // TODO: find a better solution?
       var limit = 10;
       while (tmp.length !== items.length) {
-          walk(traverseCallback(value.items || sample, path, resolve));
+          walk(traverseCallback(value.items || sample, path, resolve, null, seenSchemaCache));
           if (!limit--) {
               break;
           }
       }
       return tmp;
   }
   // TODO provide types
-  var arrayType = function arrayType(value, path, resolve, traverseCallback) {
+  var arrayType = function arrayType(value, path, resolve, traverseCallback, seenSchemaCache) {
       var items = [];
       if (!(value.items || value.additionalItems)) {
           if (utils.hasProperties(value, 'minItems', 'maxItems', 'uniqueItems')) {
@@ -24136,7 +24137,7 @@ function extend() {
       if (tmpItems instanceof Array) {
           return Array.prototype.concat.call(items, tmpItems.map(function (item, key) {
               var itemSubpath = path.concat(['items', key + '']);
-              return traverseCallback(item, itemSubpath, resolve);
+              return traverseCallback(item, itemSubpath, resolve, null, seenSchemaCache);
           }));
       }
       var minItems = value.minItems;
@@ -24165,11 +24166,11 @@ function extend() {
       sample = typeof value.additionalItems === 'object' ? value.additionalItems : {};
       for (var current = items.length; current < length; current++) {
           var itemSubpath = path.concat(['items', current + '']);
-          var element = traverseCallback(value.items || sample, itemSubpath, resolve);
+          var element = traverseCallback(value.items || sample, itemSubpath, resolve, null, seenSchemaCache);
           items.push(element);
       }
       if (value.uniqueItems) {
-          return unique(path.concat(['items']), items, value, sample, resolve, traverseCallback);
+          return unique(path.concat(['items']), items, value, sample, resolve, traverseCallback, seenSchemaCache);
       }
       return items;
   };
@@ -24233,7 +24234,7 @@ function extend() {
   var anyType = { type: ['string', 'number', 'integer', 'boolean'] };
   // TODO provide types
   // Updated objectType definition to latest version (0.5.0-rcv.41)
-  var objectType = function objectType(value, path, resolve, traverseCallback) {
+  var objectType = function objectType(value, path, resolve, traverseCallback, seenSchemaCache) {
     const props = {};
 
     const properties = value.properties || {};
@@ -24269,7 +24270,7 @@ function extend() {
         }
       });
 
-      return traverseCallback(props, path.concat(['properties']), resolve, value);
+      return traverseCallback(props, path.concat(['properties']), resolve, value, seenSchemaCache);
     }
 
     const optionalsProbability = optionAPI('alwaysFakeOptionals') === true ? 1.0 : optionAPI('optionalsProbability');
@@ -24325,7 +24326,7 @@ function extend() {
 
         return traverseCallback({
           allOf: _defns.concat(value),
-        }, path.concat(['properties']), resolve, value);
+        }, path.concat(['properties']), resolve, value, seenSchemaCache);
       }
     }
 
@@ -24478,7 +24479,7 @@ function extend() {
       }
     }
 
-    return traverseCallback(props, path.concat(['properties']), resolve, value);
+    return traverseCallback(props, path.concat(['properties']), resolve, value, seenSchemaCache);
   };
 
   /**
@@ -24664,31 +24665,42 @@ function extend() {
   };
 
   // TODO provide types
-  function traverse(schema, path, resolve, rootSchema) {
+  function traverse(schema, path, resolve, rootSchema, seenSchemaCache) {
       schema = resolve(schema);
       if (!schema) {
-          return;
+        return;
       }
       if (optionAPI('useExamplesValue') && 'example' in schema) {
         var clonedSchema,
-          result;
-
-        // avoid minItems and maxItems while checking for valid examples
-        if (optionAPI('avoidExampleItemsLength') && _.get(schema, 'type') === 'array') {
-          clonedSchema = _.clone(schema);
-          _.unset(clonedSchema, 'minItems');
-          _.unset(clonedSchema, 'maxItems');
+          result,
+          isExampleValid,
+          hashSchema = hash(schema);
 
-          // avoid validation of values that are in pm variable format (i.e. '{{userId}}')
-          result = validateSchema(clonedSchema, schema.example, { ignoreUnresolvedVariables: true });
+        if(seenSchemaCache && seenSchemaCache.has(hashSchema)) {
+          isExampleValid = seenSchemaCache.get(hashSchema);
         }
         else {
-          // avoid validation of values that are in pm variable format (i.e. '{{userId}}')
-          result = validateSchema(schema, schema.example, { ignoreUnresolvedVariables: true });
+          // avoid minItems and maxItems while checking for valid examples
+          if (optionAPI('avoidExampleItemsLength') && _.get(schema, 'type') === 'array') {
+            clonedSchema = _.clone(schema);
+            _.unset(clonedSchema, 'minItems');
+            _.unset(clonedSchema, 'maxItems');
+
+            // avoid validation of values that are in pm variable format (i.e. '{{userId}}')
+            result = validateSchema(clonedSchema, schema.example, { ignoreUnresolvedVariables: true });
+          }
+          else {
+            // avoid validation of values that are in pm variable format (i.e. '{{userId}}')
+            result = validateSchema(schema, schema.example, { ignoreUnresolvedVariables: true });
+          }
+
+          // Store the final result that needs to be used in the seen map
+          isExampleValid = result && result.length === 0;
+          seenSchemaCache && seenSchemaCache.set(hashSchema, isExampleValid);
         }
 
         // Use example only if valid
-        if (result && result.length === 0) {
+        if (isExampleValid) {
           return schema.example;
         }
       }
@@ -24707,7 +24719,7 @@ function extend() {
       }
       // thunks can return sub-schemas
       if (typeof schema.thunk === 'function') {
-          return traverse(schema.thunk(), path, resolve);
+          return traverse(schema.thunk(), path, resolve, null, seenSchemaCache);
       }
       if (typeof schema.generate === 'function') {
           return utils.typecast(schema, function () { return schema.generate(rootSchema); });
@@ -24735,7 +24747,7 @@ function extend() {
           }
           else {
               try {
-                  var result = typeMap[type](schema, path, resolve, traverse);
+                  var result = typeMap[type](schema, path, resolve, traverse, seenSchemaCache);
                   var required = schema.items
                       ? schema.items.required
                       : schema.required;
@@ -24755,7 +24767,7 @@ function extend() {
       }
       for (var prop in schema) {
           if (typeof schema[prop] === 'object' && prop !== 'definitions') {
-              copy[prop] = traverse(schema[prop], path.concat([prop]), resolve, copy);
+              copy[prop] = traverse(schema[prop], path.concat([prop]), resolve, copy, seenSchemaCache);
           }
           else {
               copy[prop] = schema[prop];
@@ -24825,7 +24837,7 @@ function extend() {
       return obj;
   }
   // TODO provide types
-  function run(refs, schema, container) {
+  function run(refs, schema, container, seenSchemaCache) {
       try {
           var result = traverse(schema, [], function reduce(sub, maxReduceDepth) {
               if (typeof maxReduceDepth === 'undefined') {
@@ -24894,7 +24906,7 @@ function extend() {
                   }
               }
               return container.wrap(sub);
-          });
+          }, null, seenSchemaCache);
           if (optionAPI('resolveJsonPath')) {
               return resolve(result);
           }
@@ -24936,7 +24948,7 @@ function extend() {
           }
       }
   }
-  var jsf = function (schema, refs) {
+  var jsf = function (schema, refs, seenSchemaCache) {
       var ignore = optionAPI('ignoreMissingRefs');
       var $ = deref(function (id, refs) {
           // FIXME: allow custom callback?
@@ -24945,7 +24957,7 @@ function extend() {
           }
       });
       var $refs = getRefs(refs);
-      return run($refs, $(schema, $refs, true), container);
+      return run($refs, $(schema, $refs, true), container, seenSchemaCache);
   };
   jsf.resolve = function (schema, refs, cwd) {
       if (typeof refs === 'string') {

lib/deref.js

@@ -149,7 +149,7 @@ module.exports = {
    */
 
   resolveRefs: function (schema, parameterSourceOption, components, schemaResolutionCache,
-    resolveFor = 'CONVERSION', resolveTo = 'schema', stack = 0, seenRef = {}, stackLimit = 8, isAllOf = false) {
+    resolveFor = 'CONVERSION', resolveTo = 'schema', stack = 0, seenRef = {}, stackLimit = 10, isAllOf = false) {
     var resolvedSchema, prop, splitRef,
       ERR_TOO_MANY_LEVELS = '<Error: Too many levels of nesting to fake this schema>';
     let concreteUtils = components && components.hasOwnProperty('concreteUtils') ?

lib/options.js

@@ -155,7 +155,7 @@ module.exports = {
         name: 'Schema resolution nesting limit',
         id: 'stackLimit',
         type: 'integer',
-        default: 8,
+        default: 10,
         description: 'Number of nesting limit till which schema resolution will happen. Increasing this limit may' +
           ' result in more time to convert collection depending on complexity of specification. (To make sure this' +
           ' option works correctly "optimizeConversion" option needs to be disabled)',

lib/schemaUtils.js

@@ -218,7 +218,7 @@ function safeSchemaFaker(oldSchema, resolveTo, resolveFor, parameterSourceOption
       return fakedSchema;
     }
     // for JSON, the indentCharacter will be applied in the JSON.stringify step later on
-    fakedSchema = schemaFaker(resolvedSchema);
+    fakedSchema = schemaFaker(resolvedSchema, null, _.get(schemaCache, 'schemaValidationCache'));
     schemaFakerCache[key] = fakedSchema;
     return fakedSchema;
   }
@@ -310,7 +310,7 @@ module.exports = {
 
     var computedOptions = _.clone(options);
 
-    computedOptions.stackLimit = 8;
+    computedOptions.stackLimit = 10;
     // This is the score that is given to each spec on the basis of the
     // number of references present in spec and the number of requests that will be generated.
     // This ranges from 0-10.

lib/schemapack.js

@@ -46,6 +46,7 @@ class SchemaPack {
     this.computedOptions = null;
     this.schemaFakerCache = {};
     this.schemaResolutionCache = {};
+    this.schemaValidationCache = new Map();
 
     this.computedOptions = utils.mergeOptions(
       // predefined options
@@ -260,7 +261,8 @@ class SchemaPack {
       authHelper,
       schemaCache = {
         schemaResolutionCache: this.schemaResolutionCache,
-        schemaFakerCache: this.schemaFakerCache
+        schemaFakerCache: this.schemaFakerCache,
+        schemaValidationCache: this.schemaValidationCache
       };
 
     if (!this.validated) {