Skip to content

Commit

Permalink
feature: lazy resolver (swagger-api#1681)
Browse files Browse the repository at this point in the history
* Don't display file name in File menu
* Remove unused `fileName` variable
* v3.2.9
* fix dropzone sizing issue
* Rebuild dist
* Filter out non-semantic errors in resolver-vulnerable test
* Point validation to unresolved spec
* Elmiinate usage and references to resolved spec
* Migrate editor validation to resolvedless strategy
* Remove debugging console points
* Add async/await support to babel
  • Loading branch information
shockey authored Feb 24, 2018
1 parent baae6c3 commit f0e8eb5
Show file tree
Hide file tree
Showing 14 changed files with 87 additions and 74 deletions.
2 changes: 2 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"stage-0"
],
"plugins": [
"transform-runtime",
"transform-async-to-generator",
[
"module-alias",
[
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
"babel-eslint": "^6.1.2",
"babel-loader": "^6.3.2",
"babel-plugin-module-alias": "^1.6.0",
"babel-plugin-transform-async-to-generator": "^6.24.1",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-es2015": "^6.22.0",
"babel-preset-es2015-ie": "^6.6.2",
"babel-preset-react": "^6.23.0",
Expand Down
10 changes: 5 additions & 5 deletions src/plugins/validate-base/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Base validate plugin that provides a placeholder `validateSpec` that fires
// after `updateResolved` is dispatched.
// after `updateJsonSpec` is dispatched.

export const updateResolved = (ori, {specActions}) => (...args) => {
export const updateJsonSpec = (ori, {specActions}) => (...args) => {
ori(...args)
/*
To allow us to remove this, we should prefer the practice of
only _composing_ inside of wrappedActions. It keeps us free to
remove pieces added by plugins. In this case, I want to toggle validation.
But I can't look inside the updateResolved action,
But I can't look inside the updateJsonSpec action,
I can only remove it ( which isn't desirable ). However I _can_ remove `validateSpec` action,
making it a noop. That way, the only overhead I end up with is a bunch of noops inside wrappedActions.
Which isn't bad.
Expand All @@ -17,7 +17,7 @@ export const updateResolved = (ori, {specActions}) => (...args) => {
}

//eslint-disable-next-line no-unused-vars
export const validateSpec = (resolvedSpec) => ({ specSelectors, errActions }) => {
export const validateSpec = (jsSpec) => ({ specSelectors, errActions }) => {

}

Expand All @@ -29,7 +29,7 @@ export default function() {
validateSpec,
},
wrapActions: {
updateResolved
updateJsonSpec
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/validate-json-schema/apis/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { makeValidationWorker } from "../helpers"

let debouncedValidation = makeValidationWorker()

export const validateSpec = (resolvedSpec) => ({ specSelectors, errActions }) => {
export const validateSpec = (jsSpec) => ({ specSelectors, errActions }) => {
const isOAS3 = specSelectors.isOAS3 ? specSelectors.isOAS3() : false
const ourMode = isOAS3 ? "oas3" : "swagger2"
debouncedValidation({ mode: ourMode, specSelectors, errActions, resolvedSpec })
debouncedValidation({ mode: ourMode, specSelectors, errActions, jsSpec })
}

export default function() {
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/validate-json-schema/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function makeValidationWorker() {
var worker = new BareValidationWorker()
var validationWorker = new PromiseWorker(worker)

function runValidation({ specSelectors, errActions, resolvedSpec, mode }) {
function runValidation({ specSelectors, errActions, mode }) {
let specStr = specSelectors.specStr()

if(specStr.trim().length === 0) {
Expand All @@ -20,7 +20,6 @@ export function makeValidationWorker() {
return validationWorker.postMessage({
mode,
jsSpec: specSelectors.specJson().toJS(),
resolvedSpec,
specStr
}).then(function (validationErrors) {
errActions.clear({
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/validate-json-schema/validation.worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import getTimestamp from "./get-timestamp"
import registerPromiseWorker from "promise-worker/register"
import modes from "./modes"

registerPromiseWorker(function ({ jsSpec, resolvedSpec, specStr, mode }) {
registerPromiseWorker(function ({ jsSpec, specStr, mode }) {
let boundGetLineNumber = getLineNumberForPath.bind(null, specStr)

if(!modes[mode]) {
Expand All @@ -18,7 +18,6 @@ registerPromiseWorker(function ({ jsSpec, resolvedSpec, specStr, mode }) {

let inputs = {
jsSpec,
resolvedSpec,
specStr,
settings,
getLineNumberForPath: boundGetLineNumber
Expand Down
1 change: 1 addition & 0 deletions src/plugins/validate-semantic/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const debNewSpecErrBatch = debounce(() => {
obj.source = SOURCE
})
system.errActions.newSpecErrBatch(errorCollector)
delete errorCollector.system
errorCollector = [] // Clear stack
} catch(e) {
console.error(e)
Expand Down
14 changes: 13 additions & 1 deletion src/plugins/validate-semantic/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as actions from "./actions"
import traverse from "traverse"
import {createSelector} from "reselect"
import debounce from "lodash/debounce"
import memoize from "lodash/memoize"

import * as formDataValidateActions from "./validators/form-data"
import * as schemaValidateActions from "./validators/schema"
Expand All @@ -21,12 +22,13 @@ export default function SemanticValidatorsPlugin({getSystem}) {
fn: {
traverse,
traverseOnce,
memoizedResolveSubtree: makeMemoizedResolveSubtree(getSystem())
},
statePlugins: {
spec: {
selectors: {
jsonAsJS: createSelector(
state => state.get("resolved"),
state => state.get("json"),
(spec) => spec ? spec.toJS() : null
)
},
Expand Down Expand Up @@ -110,3 +112,13 @@ function makeTraverseOnce(getSystem) {
return deferred.promise.then( a => a[name] )
}
}

function makeMemoizedResolveSubtree(system) {
const cacheKeymaker = (obj, path) => {
return `${obj.toString()} ${path.join("<>")}`
}
return memoize(async (obj, path, opts) => {
const res = await system.fn.resolveSubtree(obj.toJS(), path, opts)
return res
}, cacheKeymaker)
}
4 changes: 1 addition & 3 deletions src/plugins/validate-semantic/validators/operations.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ export const validateOperationHasUniqueId = () => sys => {
return nodes.reduce((acc, node) => {
const value = node.node

// Swagger-Client messes with `value.operationId`, but puts the inital
// value in `__originalOperationId` when it resolves, so we use that.
const id = value.__originalOperationId
const id = value.operationId

if(id) {
if(seen.indexOf(id) > -1) {
Expand Down
68 changes: 33 additions & 35 deletions src/plugins/validate-semantic/validators/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,42 @@ const operationKeys = ["get", "post", "put", "delete", "options", "head", "patch

const PATH_TEMPLATES_REGEX = /\{(.*?)\}/g

export const validatePathParameterDeclarationHasMatchingDefiniton = () => system => {
return system.validateSelectors
.allPathItems()
.then(nodes => {
return nodes.reduce((acc, node) => {
const pathItem = node.node
export const validatePathParameterDeclarationHasMatchingDefiniton = () => async system => {
const nodes = await system.validateSelectors.allPathItems()

return nodes.reduce(async (prev, node) => {
const acc = await prev
const pathTemplates = (node.key.match(PATH_TEMPLATES_REGEX) || [])
.map(str => str.replace("{", "").replace("}", ""))
if(pathTemplates.length) {
for (let paramName of pathTemplates) {
if(paramName.length === 0) {
// don't validate empty param names... they're invalid anyway
continue
}
const resolverResult = await system.fn.memoizedResolveSubtree(system.specSelectors.specJson(), node.path)
const res = checkForDefinition(paramName, resolverResult.spec)
if(res.inOperation && res.missingFromOperations.length) {
const missingStr = res.missingFromOperations
.map(str => `"${str}"`)
.join(", ")

const pathTemplates = (node.key.match(PATH_TEMPLATES_REGEX) || [])
.map(str => str.replace("{", "").replace("}", ""))
if(pathTemplates.length) {
pathTemplates.forEach(paramName => {
if(paramName.length === 0) {
// don't validate empty param names... they're invalid anyway
return
}
const res = checkForDefinition(paramName, pathItem)
if(res.inOperation && res.missingFromOperations.length) {
const missingStr = res.missingFromOperations
.map(str => `"${str}"`)
.join(", ")

acc.push({
message: `Declared path parameter "${paramName}" needs to be defined within every operation in the path (missing in ${missingStr}), or moved to the path-level parameters object`,
path: [...node.path],
level: "error",
})
} else if(!res.found) {
acc.push({
message: `Declared path parameter "${paramName}" needs to be defined as a path parameter at either the path or operation level`,
path: [...node.path],
level: "error",
})
}
acc.push({
message: `Declared path parameter "${paramName}" needs to be defined within every operation in the path (missing in ${missingStr}), or moved to the path-level parameters object`,
path: [...node.path],
level: "error",
})
} else if(!res.found) {
acc.push({
message: `Declared path parameter "${paramName}" needs to be defined as a path parameter at either the path or operation level`,
path: [...node.path],
level: "error",
})
}
return acc
}, [])
})
}
}
return acc
}, Promise.resolve([]))
}

export const validatePathParameterDeclarationIsNotEmpty = () => system => {
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/validate-semantic/validators/refs.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import get from "lodash/get"

export const validateRefHasNoSiblings = () => system => {
return system.validateSelectors.all$refArtifacts()
return system.validateSelectors.all$refs()
.then((nodes) => {
const immSpecJson = system.specSelectors.specJson()
const specJson = immSpecJson.toJS ? immSpecJson.toJS() : {}
Expand All @@ -27,7 +27,7 @@ export const validateRefHasNoSiblings = () => system => {

// Add warnings for unused definitions
export const validateUnusedDefinitions = () => (system) => {
return system.validateSelectors.all$refArtifacts()
return system.validateSelectors.all$refs()
.then((nodes) => {
const references = nodes.map(node => node.node)
const errors = []
Expand All @@ -49,7 +49,7 @@ export const validateUnusedDefinitions = () => (system) => {
}

export const validateRefPathFormatting = () => (system) => {
return system.validateSelectors.all$refArtifacts()
return system.validateSelectors.all$refs()
.then((refArtifacts) => {

const errors = []
Expand Down
17 changes: 8 additions & 9 deletions test/plugins/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ function getSystem(spec) {
const system = SwaggerUi({
spec,
domNode: null,
presets: [],
initialState: {
layout: undefined
},
plugins: [
presets: [
SwaggerUi.plugins.SpecIndex,
SwaggerUi.plugins.ErrIndex,
SwaggerUi.plugins.DownloadUrl,
SwaggerUi.plugins.SwaggerJsIndex,
],
initialState: {
layout: undefined
},
plugins: [
ASTPlugin,
ValidateBasePlugin,
ValidateSemanticPlugin,
Expand All @@ -27,7 +28,7 @@ function getSystem(spec) {
})
}

describe("plugin - selectors", function() {
describe("validation plugin - selectors", function() {
this.timeout(10 * 1000)

it("allSchemas should pick up parameter schemas", () => {
Expand All @@ -49,11 +50,9 @@ describe("plugin - selectors", function() {
return getSystem(spec)
.then(system => system.validateSelectors.allSchemas())
.then(nodes => {
expect(nodes.length).toEqual(3) // the "common" parameter gets propagated to "get" method
expect(nodes.length).toEqual(2)
expect(nodes[0].path).toEqual(["paths","test","parameters","0"])
expect(nodes[1].path).toEqual(["paths","test","get","parameters","0"])
expect(nodes[2].path).toEqual(["paths","test","get","parameters","1"])
expect(nodes[2].node).toEqual({name: "common"})
})
})

Expand Down
10 changes: 5 additions & 5 deletions test/plugins/validate-semantic/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,22 +312,22 @@ describe("validation plugin - semantic - paths", function(){
it("should return no problems for a templated and double-templated set of path strings", function(){
const spec = {
paths: {
"/CoolPath/{group_id}/all": {
"/CoolPath/{group_id1}/all": {
parameters: [{
name: "group_id",
name: "group_id1",
in: "path",
required: true
}]
},
"/CoolPath/{group_id}/{user_id}": {
"/CoolPath/{group_id2}/{user_id2}": {
parameters: [
{
name: "group_id",
name: "group_id2",
in: "path",
required: true
},
{
name: "user_id",
name: "user_id2",
in: "path",
required: true
},
Expand Down
17 changes: 10 additions & 7 deletions test/plugins/validate-semantic/validate-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,33 @@ import ValidateBasePlugin from "plugins/validate-base"
import ValidateSemanticPlugin from "plugins/validate-semantic"
import ASTPlugin from "plugins/ast"

const DELAY_MS = process.env.CI === "true" ? 110 : 60
const DELAY_MS = 130

export default function validateHelper(spec) {
return new Promise((resolve) => {
const system = SwaggerUi({
spec,
domNode: null,
presets: [],
initialState: {
layout: undefined
},
plugins: [
presets: [
SwaggerUi.plugins.SpecIndex,
SwaggerUi.plugins.ErrIndex,
SwaggerUi.plugins.DownloadUrl,
SwaggerUi.plugins.SwaggerJsIndex,
],
initialState: {
layout: undefined
},
plugins: [
ASTPlugin,
ValidateBasePlugin,
ValidateSemanticPlugin,

]
})
system.validateActions.all()
setTimeout(resolve.bind(null, system), DELAY_MS)
setTimeout(() => {
resolve(system)
}, DELAY_MS)
})

}
Expand Down

0 comments on commit f0e8eb5

Please sign in to comment.