diff --git a/src/allOf.js b/src/allOf.js index 83c4f80..aa031ad 100644 --- a/src/allOf.js +++ b/src/allOf.js @@ -1,12 +1,12 @@ import { traverse } from './traverse'; import { mergeDeep } from './utils'; -export function allOfSample(into, children, options, spec, context) { - let res = traverse(into, options, spec); +export function allOfSample(into, children, options, spec, context, markForRemoval) { + let res = traverse(into, options, spec, null, markForRemoval); const subSamples = []; for (let subSchema of children) { - const { type, readOnly, writeOnly, value } = traverse({ type: res.type, ...subSchema }, options, spec, context); + const { type, readOnly, writeOnly, value } = traverse({ type: res.type, ...subSchema }, options, spec, context, markForRemoval); if (res.type && type && type !== res.type) { console.warn('allOf: schemas with different types can\'t be merged'); res.type = type; diff --git a/src/openapi-sampler.js b/src/openapi-sampler.js index ff4f828..4f6bb70 100644 --- a/src/openapi-sampler.js +++ b/src/openapi-sampler.js @@ -1,5 +1,6 @@ import { traverse, clearCache } from './traverse'; import { sampleArray, sampleBoolean, sampleNumber, sampleObject, sampleString } from './samplers/index'; +import {removeForRemovalMarkedProperties} from './utils'; export var _samplers = {}; @@ -11,7 +12,9 @@ const defaults = { export function sample(schema, options, spec) { let opts = Object.assign({}, defaults, options); clearCache(); - return traverse(schema, opts, spec).value; + + let traverseResult = traverse(schema, opts, spec, null, true); + return removeForRemovalMarkedProperties(traverseResult.value); }; export function _registerSampler(type, sampler) { diff --git a/src/samplers/array.js b/src/samplers/array.js index dbd4c43..72c1d7b 100644 --- a/src/samplers/array.js +++ b/src/samplers/array.js @@ -1,5 +1,5 @@ import { traverse } from '../traverse'; -export function sampleArray(schema, options = {}, spec, context) { +export function sampleArray(schema, options = {}, spec, context, markForRemoval) { const depth = (context && context.depth || 1); let arrayLength = Math.min(schema.maxItems != undefined ? schema.maxItems : Infinity, schema.minItems || 1); @@ -21,7 +21,7 @@ export function sampleArray(schema, options = {}, spec, context) { for (let i = 0; i < arrayLength; i++) { let itemSchema = itemSchemaGetter(i); - let { value: sample } = traverse(itemSchema, options, spec, {depth: depth + 1}); + let { value: sample } = traverse(itemSchema, options, spec, {depth: depth + 1}, markForRemoval); res.push(sample); } return res; diff --git a/src/samplers/object.js b/src/samplers/object.js index 890057f..97fe643 100644 --- a/src/samplers/object.js +++ b/src/samplers/object.js @@ -1,5 +1,7 @@ import { traverse } from '../traverse'; -export function sampleObject(schema, options = {}, spec, context) { +import {MARKED_FOR_REMOVAL} from '../utils'; + +export function sampleObject(schema, options = {}, spec, context, markForRemoval = false) { let res = {}; const depth = (context && context.depth || 1); @@ -13,15 +15,25 @@ export function sampleObject(schema, options = {}, spec, context) { Object.keys(schema.properties).forEach(propertyName => { // skip before traverse that could be costly if (options.skipNonRequired && !requiredKeyDict.hasOwnProperty(propertyName)) { + if(markForRemoval) { + res[propertyName] = MARKED_FOR_REMOVAL; + } return; } - const sample = traverse(schema.properties[propertyName], options, spec, { propertyName, depth: depth + 1 }); + const sample = traverse(schema.properties[propertyName], options, spec, { propertyName, depth: depth + 1 }, markForRemoval); + if (options.skipReadOnly && sample.readOnly) { + if(markForRemoval) { + res[propertyName] = MARKED_FOR_REMOVAL; + } return; } if (options.skipWriteOnly && sample.writeOnly) { + if(markForRemoval) { + res[propertyName] = MARKED_FOR_REMOVAL; + } return; } res[propertyName] = sample.value; @@ -30,8 +42,8 @@ export function sampleObject(schema, options = {}, spec, context) { if (schema && typeof schema.additionalProperties === 'object') { const propertyName = schema.additionalProperties['x-additionalPropertiesName'] || 'property'; - res[`${String(propertyName)}1`] = traverse(schema.additionalProperties, options, spec, {depth: depth + 1 }).value; - res[`${String(propertyName)}2`] = traverse(schema.additionalProperties, options, spec, {depth: depth + 1 }).value; + res[`${String(propertyName)}1`] = traverse(schema.additionalProperties, options, spec, {depth: depth + 1 }, markForRemoval).value; + res[`${String(propertyName)}2`] = traverse(schema.additionalProperties, options, spec, {depth: depth + 1 }, markForRemoval).value; } return res; } diff --git a/src/traverse.js b/src/traverse.js index 96f248a..b71cbb3 100644 --- a/src/traverse.js +++ b/src/traverse.js @@ -1,7 +1,7 @@ import { _samplers } from './openapi-sampler'; import { allOfSample } from './allOf'; import { inferType } from './infer'; -import { getResultForCircular, mergeDeep, popSchemaStack } from './utils'; +import {getResultForCircular, mergeDeep, popSchemaStack, filterDeep} from './utils'; import JsonPointer from 'json-pointer'; let $refCache = {}; @@ -13,12 +13,45 @@ export function clearCache() { seenSchemasStack = []; } -function inferExample(schema) { +/** + * Takes an example object of a schema and makes sure it is valid with the + * supplied schema by removing invalid properties. + * + * @param schema the schema to check and make the example valid with + * @param example the example of the schema to use as the base and output a valid example + * @param options the sampling options + * @param spec the whole openapi spec + * @param markForRemoval whether properties should be marked for removal because they should not be in the example + * @returns valid example + */ +function tryMakeExampleObjectValid(schema, example, options, spec, markForRemoval) { + if (typeof schema !== 'object') { + return example; + } + if (typeof example !== 'object') { + return example; + } + + let exampleLessSchema = Object.assign({}, schema); + delete exampleLessSchema.example; // required to remove for traverse + delete exampleLessSchema.examples; // required to remove for traverse + + let value = traverse(exampleLessSchema, options, spec, null, markForRemoval).value; + let oldValue = value; + + value = mergeDeep(value, example); + // Remove all example properties which are not baked by a + // property in the schema + value = filterDeep(oldValue, value) + return value; +} + +function inferExample(schema, options, spec, markForRemoval) { let example; if (schema.const !== undefined) { example = schema.const; } else if (schema.examples !== undefined && schema.examples.length) { - example = schema.examples[0]; + example = tryMakeExampleObjectValid(schema, schema.examples[0], options, spec, markForRemoval); } else if (schema.enum !== undefined && schema.enum.length) { example = schema.enum[0]; } else if (schema.default !== undefined) { @@ -27,8 +60,8 @@ function inferExample(schema) { return example; } -function tryInferExample(schema) { - const example = inferExample(schema); +function tryInferExample(schema, options, spec, markForRemoval) { + const example = inferExample(schema, options, spec, markForRemoval); // case when we don't infer example from schema but take from `const`, `examples`, `default` or `enum` keywords if (example !== undefined) { return { @@ -41,7 +74,7 @@ function tryInferExample(schema) { return; } -export function traverse(schema, options, spec, context) { +export function traverse(schema, options, spec, context, markForRemoval = false) { // checking circular JS references by checking context // because context is passed only when traversing through nested objects happens if (context) { @@ -69,7 +102,7 @@ export function traverse(schema, options, spec, context) { if ($refCache[ref] !== true) { $refCache[ref] = true; - result = traverse(referenced, options, spec, context); + result = traverse(referenced, options, spec, context, markForRemoval); $refCache[ref] = false; } else { const referencedType = inferType(referenced); @@ -80,9 +113,11 @@ export function traverse(schema, options, spec, context) { } if (schema.example !== undefined) { + let value = tryMakeExampleObjectValid(schema, schema.example, options, spec, markForRemoval); + popSchemaStack(seenSchemasStack, context); return { - value: schema.example, + value: value, readOnly: schema.readOnly, writeOnly: schema.writeOnly, type: schema.type, @@ -91,12 +126,13 @@ export function traverse(schema, options, spec, context) { if (schema.allOf !== undefined) { popSchemaStack(seenSchemasStack, context); - return tryInferExample(schema) || allOfSample( + return tryInferExample(schema, options, spec, markForRemoval) || allOfSample( { ...schema, allOf: undefined }, schema.allOf, options, spec, context, + markForRemoval ); } @@ -130,10 +166,10 @@ export function traverse(schema, options, spec, context) { if (schema.if && schema.then) { popSchemaStack(seenSchemasStack, context); const { if: ifSchema, then, ...rest } = schema; - return traverse(mergeDeep(rest, ifSchema, then), options, spec, context); + return traverse(mergeDeep(rest, ifSchema, then), options, spec, context, markForRemoval); } - let example = inferExample(schema); + let example = inferExample(schema, options, spec, markForRemoval); let type = null; if (example === undefined) { example = null; @@ -146,7 +182,7 @@ export function traverse(schema, options, spec, context) { } let sampler = _samplers[type]; if (sampler) { - example = sampler(schema, options, spec, context); + example = sampler(schema, options, spec, context, markForRemoval); } } @@ -159,13 +195,13 @@ export function traverse(schema, options, spec, context) { }; function traverseOneOrAnyOf(schema, selectedSubSchema) { - const inferred = tryInferExample(schema); + const inferred = tryInferExample(schema, options, spec, markForRemoval); if (inferred !== undefined) { return inferred; } - const localExample = traverse({...schema, oneOf: undefined, anyOf: undefined }, options, spec, context); - const subSchemaExample = traverse(selectedSubSchema, options, spec, context); + const localExample = traverse({...schema, oneOf: undefined, anyOf: undefined }, options, spec, context, markForRemoval); + const subSchemaExample = traverse(selectedSubSchema, options, spec, context, markForRemoval); if (typeof localExample.value === 'object' && typeof subSchemaExample.value === 'object') { const mergedExample = mergeDeep(localExample.value, subSchemaExample.value); diff --git a/src/utils.js b/src/utils.js index 04f90aa..54d06c1 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,5 +1,10 @@ 'use strict'; +export const MARKED_FOR_REMOVAL = { + type: 'REDOCLY_INTERNAL_MARKED_FOR_REMOVAL', + time: new Date() // some unique identifier +}; + function pad(number) { if (number < 10) { return '0' + number; @@ -7,6 +12,81 @@ function pad(number) { return number; } +/** + * Removes all properties which have MARKED_FOR_REMOVAL as their reference. + * + * Samplers mark properties for removal depending on the configuration + * (skipNonRequired=true, skipReadOnly=true, skipWriteOnly=true). This is necessary as the + * mentioned configurations are otherwise lost when the object is a direct or indirect child + * of an allOf block. + */ +export function removeForRemovalMarkedProperties(sample) { + if (sample !== null && sample !== undefined && typeof sample === 'object') { + Object.keys(sample).forEach(key => { + if (sample[key] !== undefined && typeof sample[key] === 'object') { + if (sample[key] === MARKED_FOR_REMOVAL) { + delete sample[key]; + return sample; + } + return removeForRemovalMarkedProperties(sample[key]); + } + }); + } + return sample; +}; + +/** + * Filters an objects keys to only include keys which are present in + * a blueprint object. + * + * @param blueprint reference object which holds all allowed keys + * @param check object for which keys are filtered by the blueprint + * @returns filtered object + */ +export function filterDeep(blueprint, check) { + // filter out invalid blueprints + if (blueprint === undefined || blueprint === null || !Object.keys(blueprint).length) { + return check; + } + // If blueprint is: + // * array of single value + // * value is a primitive type + // and check value is: + // * list of primitive types with same type + // + // return as valid for now + // TODO implement checks for every primitive type + if(Array.isArray(blueprint) && blueprint.length === 1 && isPrimitive(blueprint[0])) { + if(!Array.isArray(check)) { + // If value to be checked is no array, return nothing + return blueprint; + } + // Check if every value in array has same type + if(!check.every( (val, i, arr) => typeof val === typeof arr[0])) { + return blueprint; + } + + return check; + } + + return Object.assign(...Object.keys(blueprint).map(key => { + if (typeof blueprint[key] === 'object' && typeof check[key] === 'object') { + if(check[key] === MARKED_FOR_REMOVAL) { + return { [key]: check[key] } + } + + let childDeepFilter = filterDeep(blueprint[key], check[key]); + return Object.keys(childDeepFilter).length ? { [key]: childDeepFilter } : {}; + } + + if(key in check) { + return {[key]: check[key]}; + } + + return {}; + })); +} + export function toRFCDateTime(date, omitTime, omitDate, milliseconds) { var res = omitDate ? '' : (date.getUTCFullYear() + '-' + pad(date.getUTCMonth() + 1) + @@ -39,7 +119,12 @@ export function mergeDeep(...objects) { if (isObject(pVal) && isObject(oVal)) { prev[key] = mergeDeep(pVal, oVal); } else { - prev[key] = oVal; + if (prev[key] === MARKED_FOR_REMOVAL) { + // do nothing. MARKED_FOR_REMOVAL will be filtered out later + // before returning the sampling result. + } else { + prev[key] = oVal; + } } }); @@ -93,3 +178,11 @@ function jsf32(a, b, c, d) { return (d >>> 0) / 4294967296; } } + +function isPrimitive(value) { + if(value === null){ + return true; + } + + return !(typeof value == 'object' || typeof value == 'function'); +} diff --git a/test/integration.spec.js b/test/integration.spec.js index f279a90..296c843 100644 --- a/test/integration.spec.js +++ b/test/integration.spec.js @@ -947,4 +947,139 @@ describe('Integration', function() { expect(result).to.deep.equal(expected); }); }); + + it('should skip properties in example which are not part of the schema', () => { + const spec = { + components: { + schemas: { + TestSchema: { + type: 'object', + properties: { + a: {type: 'string'}, + b: {type: 'integer'} + }, + required: ['a', 'b'], + example: { + a: 'some a string', + b: 1, + c: 'some invalid property' + } + } + } + } + } + + const schema = {$ref: '#/components/schemas/TestSchema'} + + result = OpenAPISampler.sample(schema, {}, spec); + + expect(result).to.deep.equal({ + a: 'some a string', + b: 1 + }); + }); + + it('should skip nested properties in example which are not part of the schema', () => { + const spec = { + components: { + schemas: { + TestSchema: { + type: 'object', + properties: { + a: {type: 'string'}, + b: {type: 'integer'}, + c: { + type: 'object', + properties: { + d: { + type: 'string' + } + } + }, + }, + required: ['a', 'b'], + example: { + a: 'some a string', + b: 1, + c: { + d: 'some d string', + e: 'invalid e string' + } + } + } + } + } + } + + const schema = {$ref: '#/components/schemas/TestSchema'} + + result = OpenAPISampler.sample(schema, {}, spec); + + expect(result).to.deep.equal({ + a: 'some a string', + b: 1, + c: { + d: 'some d string' + } + }); + }); + + it('should skip properties in example marked as readOnly even if required with skipReadOnly=true', () => { + const spec = { + components: { + schemas: { + TestSchema: { + type: 'object', + properties: { + a: {type: 'string'}, + b: {type: 'integer', readOnly: true} + }, + required: ['a', 'b'], + example: { + a: 'some a string', + b: 1 + } + } + } + } + } + + const schema = {$ref: '#/components/schemas/TestSchema'} + + result = OpenAPISampler.sample(schema, {skipReadOnly: true}, spec); + + expect(result).to.deep.equal({ + a: 'some a string' + }); + }); + + it('should skip properties in example marked as writeOnly even if required with skipWriteOnly=true', () => { + const spec = { + components: { + schemas: { + type: 'object', + TestSchema: { + type: 'object', + properties: { + a: {type: 'string'}, + b: {type: 'integer', writeOnly: true} + }, + required: ['a', 'b'], + example: { + a: 'some a string', + b: 1 + } + } + } + } + } + + const schema = {$ref: '#/components/schemas/TestSchema'} + + result = OpenAPISampler.sample(schema, {skipWriteOnly: true}, spec); + + expect(result).to.deep.equal({ + a: 'some a string' + }); + }); }); diff --git a/test/unit/object.spec.js b/test/unit/object.spec.js index 537c629..03b3efc 100644 --- a/test/unit/object.spec.js +++ b/test/unit/object.spec.js @@ -1,4 +1,5 @@ -import { sampleObject} from '../../src/samplers/object.js'; +import {sampleObject} from '../../src/samplers'; +import {removeForRemovalMarkedProperties} from '../../src/utils'; describe('sampleObject', () => { let res; @@ -28,6 +29,19 @@ describe('sampleObject', () => { }); }); + it('should skip readonly properties even if required skipReadOnly=true', () => { + res = sampleObject({ + properties: { + a: {type: 'string'}, + b: {type: 'integer', readOnly: true} + }, + required: ['a', 'b'] + }, {skipReadOnly: true}); + expect(res).to.deep.equal({ + a: 'string' + }); + }); + it('should skip readonly properties in nested objects if skipReadOnly=true', () => { res = sampleObject({properties: { a: {type: 'string'}, @@ -80,6 +94,50 @@ describe('sampleObject', () => { }); }); + it('should skip readonly properties in allOfs if skipReadOnly=true', () => { + res = sampleObject( + { + properties: { + a: { type: 'string' }, + b: { + type: 'object', + allOf: [ + // Have some object and make specific properties readOnly for e.g. a PATCH method + { + type: 'object', + properties: { + c: { + type: 'string', + }, + d: { + type: 'string' + } + }, + }, + { + type: 'object', + properties: { + c: { + readOnly: true, + }, + }, + }, + ], + }, + }, + }, + { skipReadOnly: true }, + null, null, true + ); + res = removeForRemovalMarkedProperties(res); + expect(res).to.deep.equal({ + a: 'string', + b: { + d: 'string' + } + }); + }); + it('should skip writeonly properties if writeonly=true', () => { res = sampleObject({properties: { a: {type: 'string'}, @@ -90,6 +148,19 @@ describe('sampleObject', () => { }); }); + it('should skip writeOnly properties even if required and skipWriteOnly=true', () => { + res = sampleObject({ + properties: { + a: {type: 'string'}, + b: {type: 'integer', writeOnly: true} + }, + required: ['a', 'b'] + }, {skipWriteOnly: true}); + expect(res).to.deep.equal({ + a: 'string' + }); + }); + it('should skip writeonly properties in nested objects if writeonly=true', () => { res = sampleObject({properties: { a: {type: 'string'}, @@ -142,6 +213,51 @@ describe('sampleObject', () => { }); }); + it('should skip writeonly properties in allOfs if skipReadOnly=true', () => { + res = sampleObject( + { + properties: { + a: { type: 'string' }, + b: { + type: 'object', + allOf: [ + // Have some object and make specific properties writeOnly to hide + // them for everything except e.g. a PATCH method + { + type: 'object', + properties: { + c: { + type: 'string', + }, + d: { + type: 'string' + } + }, + }, + { + type: 'object', + properties: { + c: { + writeOnly: true, + }, + }, + }, + ], + }, + }, + }, + { skipWriteOnly: true }, + null, null, true + ); + res = removeForRemovalMarkedProperties(res); + expect(res).to.deep.equal({ + a: 'string', + b: { + d: 'string' + } + }); + }); + it('should should instantiate 2 additionalProperties', () => { res = sampleObject({additionalProperties: {type: 'string'}}); expect(res).to.deep.equal({ @@ -166,6 +282,7 @@ describe('sampleObject', () => { }, required: ['a'] }, {skipNonRequired: true}); + expect(res).to.deep.equal({ a: 'string' });