From 8e295c23a441101b62afac9ef2c53bed8604e749 Mon Sep 17 00:00:00 2001 From: kyle Date: Thu, 21 Jun 2018 21:36:38 -0700 Subject: [PATCH] Improvement: Hash-keyed Try-It-Out parameter value storage (#4670) * allow param update by identity + hashed value storage * add specActions.changeParamByIdentity * add identity-based lookup support in spec selectors * migrate `changeParam` usage to `changeParamByIdentity` * migrate usage of `parameterWithMeta` to `parameterWithMetaByIdentity` * update invocations of `changeParamByIdentity` to match fn signature * use OrderedMap throughout hash-based selectors for consistency * normalize usage of ParameterRow `onChange` * migrate bug 4557 tests to reflect new ParameterRow interface * remove exclusive test blocks * linter fixes * copy Parameters changes into OAS3 wrapper * use rawParam for meta lookups in ParameterRow --- src/core/components/param-body.jsx | 9 +- src/core/components/parameter-row.jsx | 17 +- src/core/components/parameters.jsx | 7 +- .../oas3/wrap-components/parameters.jsx | 7 +- src/core/plugins/spec/actions.js | 7 + src/core/plugins/spec/reducers.js | 13 +- src/core/plugins/spec/selectors.js | 46 +-- test/bugs/4557-default-parameter-values.js | 14 +- test/core/plugins/spec/actions.js | 25 +- test/core/plugins/spec/reducer.js | 49 ++++ test/core/plugins/spec/selectors.js | 261 ++++++++++++++++++ 11 files changed, 410 insertions(+), 45 deletions(-) diff --git a/src/core/components/param-body.jsx b/src/core/components/param-body.jsx index d1b556f6..7caed097 100644 --- a/src/core/components/param-body.jsx +++ b/src/core/components/param-body.jsx @@ -46,11 +46,10 @@ export default class ParamBody extends PureComponent { } updateValues = (props) => { - let { specSelectors, pathMethod, param, isExecute, consumesValue="" } = props - let parameter = (specSelectors ? specSelectors.parameterWithMeta(pathMethod, param.get("name"), param.get("in")) : fromJS({})) || param + let { param, isExecute, consumesValue="" } = props let isXml = /xml/i.test(consumesValue) let isJson = /json/i.test(consumesValue) - let paramValue = isXml ? parameter.get("value_xml") : parameter.get("value") + let paramValue = isXml ? param.get("value_xml") : param.get("value") if ( paramValue !== undefined ) { let val = !paramValue && isJson ? "{}" : paramValue @@ -79,7 +78,7 @@ export default class ParamBody extends PureComponent { this._onChange(value, isXml) } - _onChange = (val, isXml) => { (this.props.onChange || NOOP)(this.props.param, val, isXml) } + _onChange = (val, isXml) => { (this.props.onChange || NOOP)(val, isXml) } handleOnChange = e => { const {consumesValue} = this.props @@ -107,7 +106,7 @@ export default class ParamBody extends PureComponent { const HighlightCode = getComponent("highlightCode") const ContentType = getComponent("contentType") // for domains where specSelectors not passed - let parameter = specSelectors ? specSelectors.parameterWithMeta(pathMethod, param.get("name"), param.get("in")) : param + let parameter = specSelectors ? specSelectors.parameterWithMetaByIdentity(pathMethod, param) : param let errors = parameter.get("errors", List()) let consumesValue = specSelectors.contentTypeValues(pathMethod).get("requestContentType") let consumes = this.props.consumes && this.props.consumes.size ? this.props.consumes : ParamBody.defaultProp.consumes diff --git a/src/core/components/parameter-row.jsx b/src/core/components/parameter-row.jsx index 3ec47a55..b8ae0c62 100644 --- a/src/core/components/parameter-row.jsx +++ b/src/core/components/parameter-row.jsx @@ -9,6 +9,7 @@ export default class ParameterRow extends Component { static propTypes = { onChange: PropTypes.func.isRequired, param: PropTypes.object.isRequired, + rawParam: PropTypes.object.isRequired, getComponent: PropTypes.func.isRequired, fn: PropTypes.object.isRequired, isExecute: PropTypes.bool, @@ -30,7 +31,7 @@ export default class ParameterRow extends Component { let { isOAS3 } = specSelectors let example = param.get("example") - let parameter = specSelectors.parameterWithMeta(pathMethod, param.get("name"), param.get("in")) || param + let parameter = specSelectors.parameterWithMetaByIdentity(pathMethod, param) || param let enumValue if(isOAS3()) { @@ -56,9 +57,9 @@ export default class ParameterRow extends Component { } } - onChangeWrapper = (value) => { - let { onChange, param } = this.props - return onChange(param, value) + onChangeWrapper = (value, isXml = false) => { + let { onChange, rawParam } = this.props + return onChange(rawParam, value, isXml) } setDefaultValue = () => { @@ -72,7 +73,7 @@ export default class ParameterRow extends Component { let defaultValue = schema.get("default") let xExampleValue = param.get("x-example") // Swagger 2 only - let parameter = specSelectors.parameterWithMeta(pathMethod, param.get("name"), param.get("in")) + let parameter = specSelectors.parameterWithMetaByIdentity(pathMethod, param) let value = parameter ? parameter.get("value") : "" if( param.get("in") !== "body" ) { @@ -85,7 +86,7 @@ export default class ParameterRow extends Component { } render() { - let {param, onChange, getComponent, getConfigs, isExecute, fn, onChangeConsumes, specSelectors, pathMethod, specPath} = this.props + let {param, rawParam, getComponent, getConfigs, isExecute, fn, onChangeConsumes, specSelectors, pathMethod, specPath} = this.props let { isOAS3 } = specSelectors @@ -101,7 +102,7 @@ export default class ParameterRow extends Component { param={param} consumes={ specSelectors.operationConsumes(pathMethod) } consumesValue={ specSelectors.contentTypeValues(pathMethod).get("requestContentType") } - onChange={onChange} + onChange={this.onChangeWrapper} onChangeConsumes={onChangeConsumes} isExecute={ isExecute } specSelectors={ specSelectors } @@ -112,7 +113,7 @@ export default class ParameterRow extends Component { const Markdown = getComponent("Markdown") const ParameterExt = getComponent("ParameterExt") - let paramWithMeta = specSelectors.parameterWithMeta(pathMethod, param.get("name"), param.get("in")) + let paramWithMeta = specSelectors.parameterWithMetaByIdentity(pathMethod, rawParam) let format = param.get("format") let schema = isOAS3 && isOAS3() ? param.get("schema") : param let type = schema.get("type") diff --git a/src/core/components/parameters.jsx b/src/core/components/parameters.jsx index c5b1e229..e5ce323b 100644 --- a/src/core/components/parameters.jsx +++ b/src/core/components/parameters.jsx @@ -36,11 +36,11 @@ export default class Parameters extends Component { onChange = ( param, value, isXml ) => { let { - specActions: { changeParam }, + specActions: { changeParamByIdentity }, onChangeKey, } = this.props - changeParam( onChangeKey, param.get("name"), param.get("in"), value, isXml) + changeParamByIdentity(onChangeKey, param, value, isXml) } onChangeConsumesWrapper = ( val ) => { @@ -101,7 +101,8 @@ export default class Parameters extends Component { specPath={specPath.push(i.toString())} getComponent={ getComponent } getConfigs={ getConfigs } - param={ specSelectors.parameterWithMeta(pathMethod, parameter.get("name"), parameter.get("in")) } + rawParam={ parameter } + param={ specSelectors.parameterWithMetaByIdentity(pathMethod, parameter) } key={ `${parameter.get( "in" )}.${parameter.get("name")}` } onChange={ this.onChange } onChangeConsumes={this.onChangeConsumesWrapper} diff --git a/src/core/plugins/oas3/wrap-components/parameters.jsx b/src/core/plugins/oas3/wrap-components/parameters.jsx index 31b70f7f..220b02af 100644 --- a/src/core/plugins/oas3/wrap-components/parameters.jsx +++ b/src/core/plugins/oas3/wrap-components/parameters.jsx @@ -47,11 +47,11 @@ class Parameters extends Component { onChange = ( param, value, isXml ) => { let { - specActions: { changeParam }, + specActions: { changeParamByIdentity }, onChangeKey, } = this.props - changeParam( onChangeKey, param.get("name"), param.get("in"), value, isXml) + changeParamByIdentity( onChangeKey, param, value, isXml) } onChangeConsumesWrapper = ( val ) => { @@ -145,7 +145,8 @@ class Parameters extends Component { getComponent={ getComponent } specPath={specPath.push(i)} getConfigs={ getConfigs } - param={ parameter } + rawParam={ parameter } + param={ specSelectors.parameterWithMetaByIdentity(pathMethod, parameter) } key={ parameter.get( "name" ) } onChange={ this.onChange } onChangeConsumes={this.onChangeConsumesWrapper} diff --git a/src/core/plugins/spec/actions.js b/src/core/plugins/spec/actions.js index d35e09e5..b062ecf2 100644 --- a/src/core/plugins/spec/actions.js +++ b/src/core/plugins/spec/actions.js @@ -234,6 +234,13 @@ export function changeParam( path, paramName, paramIn, value, isXml ){ } } +export function changeParamByIdentity( pathMethod, param, value, isXml ){ + return { + type: UPDATE_PARAM, + payload:{ path: pathMethod, param, value, isXml } + } +} + export const updateResolvedSubtree = (path, value) => { return { type: UPDATE_RESOLVED_SUBTREE, diff --git a/src/core/plugins/spec/reducers.js b/src/core/plugins/spec/reducers.js index c95c7ce9..a510bb50 100644 --- a/src/core/plugins/spec/reducers.js +++ b/src/core/plugins/spec/reducers.js @@ -51,12 +51,21 @@ export default { }, [UPDATE_PARAM]: ( state, {payload} ) => { - let { path: pathMethod, paramName, paramIn, value, isXml } = payload + let { path: pathMethod, paramName, paramIn, param, value, isXml } = payload + + let paramKey + + // `hashCode` is an Immutable.js Map method + if(param && param.hashCode && !paramIn && !paramName) { + paramKey = `${param.get("name")}.${param.get("in")}.hash-${param.hashCode()}` + } else { + paramKey = `${paramName}.${paramIn}` + } const valueKey = isXml ? "value_xml" : "value" return state.setIn( - ["meta", "paths", ...pathMethod, "parameters", `${paramName}.${paramIn}`, valueKey], + ["meta", "paths", ...pathMethod, "parameters", paramKey, valueKey], value ) }, diff --git a/src/core/plugins/spec/selectors.js b/src/core/plugins/spec/selectors.js index f09f31b9..432d9698 100644 --- a/src/core/plugins/spec/selectors.js +++ b/src/core/plugins/spec/selectors.js @@ -294,34 +294,42 @@ export const allowTryItOutFor = () => { return true } -export const operationWithMeta = (state, path, method) => { - const op = specJsonWithResolvedSubtrees(state).getIn(["paths", path, method], Map()) - const meta = state.getIn(["meta", "paths", path, method], Map()) +export const parameterWithMetaByIdentity = (state, pathMethod, param) => { + const opParams = specJsonWithResolvedSubtrees(state).getIn(["paths", ...pathMethod, "parameters"], OrderedMap()) + const metaParams = state.getIn(["meta", "paths", ...pathMethod, "parameters"], OrderedMap()) - const mergedParams = op.get("parameters", List()).map((param) => { - return Map().merge( - param, - meta.getIn(["parameters", `${param.get("name")}.${param.get("in")}`]) + const mergedParams = opParams.map((currentParam) => { + const nameInKeyedMeta = metaParams.get(`${param.get("name")}.${param.get("in")}`) + const hashKeyedMeta = metaParams.get(`${param.get("name")}.${param.get("in")}.hash-${param.hashCode()}`) + return OrderedMap().merge( + currentParam, + nameInKeyedMeta, + hashKeyedMeta ) }) - return Map() - .merge(op, meta) - .set("parameters", mergedParams) + return mergedParams.find(curr => curr.get("in") === param.get("in") && curr.get("name") === param.get("name"), OrderedMap()) } -export const parameterWithMeta = (state, pathMethod, paramName, paramIn) => { - const opParams = specJsonWithResolvedSubtrees(state).getIn(["paths", ...pathMethod, "parameters"], Map()) - const metaParams = state.getIn(["meta", "paths", ...pathMethod, "parameters"], Map()) - const mergedParams = opParams.map((param) => { - return Map().merge( - param, - metaParams.get(`${param.get("name")}.${param.get("in")}`) - ) +export const parameterWithMeta = (state, pathMethod, paramName, paramIn) => { + const opParams = specJsonWithResolvedSubtrees(state).getIn(["paths", ...pathMethod, "parameters"], OrderedMap()) + const currentParam = opParams.find(param => param.get("in") === paramIn && param.get("name") === paramName, OrderedMap()) + + return parameterWithMetaByIdentity(state, pathMethod, currentParam) +} + +export const operationWithMeta = (state, path, method) => { + const op = specJsonWithResolvedSubtrees(state).getIn(["paths", path, method], OrderedMap()) + const meta = state.getIn(["meta", "paths", path, method], OrderedMap()) + + const mergedParams = op.get("parameters", List()).map((param) => { + return parameterWithMetaByIdentity(state, [path, method], param) }) - return mergedParams.find(param => param.get("in") === paramIn && param.get("name") === paramName, Map()) + return OrderedMap() + .merge(op, meta) + .set("parameters", mergedParams) } // Get the parameter value by parameter name diff --git a/test/bugs/4557-default-parameter-values.js b/test/bugs/4557-default-parameter-values.js index 20514678..aba5a7af 100644 --- a/test/bugs/4557-default-parameter-values.js +++ b/test/bugs/4557-default-parameter-values.js @@ -18,12 +18,14 @@ describe("bug #4557: default parameter values", function(){ getComponent: ()=> "div", specSelectors: { security(){}, - parameterWithMeta(){ return paramValue }, + parameterWithMetaByIdentity(){ return paramValue }, isOAS3(){ return false } }, + fn: {}, operation: {get: ()=>{}}, onChange: createSpy(), param: paramValue, + rawParam: paramValue, onChangeConsumes: () => {}, pathMethod: [], getConfigs: () => { return {} }, @@ -32,7 +34,8 @@ describe("bug #4557: default parameter values", function(){ render() - expect(props.onChange).toHaveBeenCalledWith(paramValue, "MyDefaultValue") + expect(props.onChange).toHaveBeenCalled() + expect(props.onChange).toHaveBeenCalledWith(paramValue, "MyDefaultValue", false) }) it("should apply an OpenAPI 3.0 default value", function(){ @@ -48,12 +51,14 @@ describe("bug #4557: default parameter values", function(){ getComponent: ()=> "div", specSelectors: { security(){}, - parameterWithMeta(){ return paramValue }, + parameterWithMetaByIdentity(){ return paramValue }, isOAS3(){ return true } }, + fn: {}, operation: {get: ()=>{}}, onChange: createSpy(), param: paramValue, + rawParam: paramValue, onChangeConsumes: () => {}, pathMethod: [], getConfigs: () => { return {} }, @@ -62,6 +67,7 @@ describe("bug #4557: default parameter values", function(){ render() - expect(props.onChange).toHaveBeenCalledWith(paramValue, "MyDefaultValue") + expect(props.onChange).toHaveBeenCalled() + expect(props.onChange).toHaveBeenCalledWith(paramValue, "MyDefaultValue", false) }) }) diff --git a/test/core/plugins/spec/actions.js b/test/core/plugins/spec/actions.js index 58aec54e..47dd7955 100644 --- a/test/core/plugins/spec/actions.js +++ b/test/core/plugins/spec/actions.js @@ -1,7 +1,7 @@ /* eslint-env mocha */ import expect, { createSpy } from "expect" import { fromJS } from "immutable" -import { execute, executeRequest } from "corePlugins/spec/actions" +import { execute, executeRequest, changeParamByIdentity } from "corePlugins/spec/actions" describe("spec plugin - actions", function(){ @@ -184,4 +184,27 @@ describe("spec plugin - actions", function(){ it.skip("should call errActions.newErr, if the fn.execute rejects", function(){ }) + describe("changeParamByIdentity", function () { + it("should map its arguments to a payload", function () { + const pathMethod = ["/one", "get"] + const param = fromJS({ + name: "body", + in: "body" + }) + const value = "my value" + const isXml = false + + const result = changeParamByIdentity(pathMethod, param, value, isXml) + + expect(result).toEqual({ + type: "spec_update_param", + payload: { + path: pathMethod, + param, + value, + isXml + } + }) + }) + }) }) diff --git a/test/core/plugins/spec/reducer.js b/test/core/plugins/spec/reducer.js index 1c91e054..89de197a 100644 --- a/test/core/plugins/spec/reducer.js +++ b/test/core/plugins/spec/reducer.js @@ -129,4 +129,53 @@ describe("spec plugin - reducer", function(){ expect(response).toEqual(expectedResult) }) }) + describe("SPEC_UPDATE_PARAM", function() { + it("should store parameter values by name+in", () => { + const updateParam = reducer["spec_update_param"] + + const path = "/pet/post" + const method = "POST" + + const state = fromJS({}) + const result = updateParam(state, { + payload: { + path: [path, method], + paramName: "body", + paramIn: "body", + value: `{ "a": 123 }`, + isXml: false + } + }) + + const response = result.getIn(["meta", "paths", path, method, "parameters", "body.body", "value"]) + expect(response).toEqual(`{ "a": 123 }`) + }) + it("should store parameter values by identity", () => { + const updateParam = reducer["spec_update_param"] + + const path = "/pet/post" + const method = "POST" + + const param = fromJS({ + name: "body", + in: "body", + schema: { + type: "string" + } + }) + + const state = fromJS({}) + const result = updateParam(state, { + payload: { + param, + path: [path, method], + value: `{ "a": 123 }`, + isXml: false + } + }) + + const value = result.getIn(["meta", "paths", path, method, "parameters", `body.body.hash-${param.hashCode()}`, "value"]) + expect(value).toEqual(`{ "a": 123 }`) + }) + }) }) diff --git a/test/core/plugins/spec/selectors.js b/test/core/plugins/spec/selectors.js index 139d4600..39483171 100644 --- a/test/core/plugins/spec/selectors.js +++ b/test/core/plugins/spec/selectors.js @@ -11,6 +11,11 @@ import { } from "corePlugins/spec/selectors" import Petstore from "./assets/petstore.json" +import { + operationWithMeta, + parameterWithMeta, + parameterWithMetaByIdentity +} from "../../../../src/core/plugins/spec/selectors" describe("spec plugin - selectors", function(){ @@ -451,4 +456,260 @@ describe("spec plugin - selectors", function(){ expect(result.getIn(["paths"]).keySeq().toJS()).toEqual(correctOrder) }) }) + + describe("operationWithMeta", function() { + it("should support merging in name+in keyed param metadata", function () { + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [ + { + name: "body", + in: "body" + } + ] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + "body.body": { + value: "abc123" + } + } + } + } + } + } + }) + + const result = operationWithMeta(state, "/", "get") + + expect(result.toJS()).toEqual({ + parameters: [ + { + name: "body", + in: "body", + value: "abc123" + } + ] + }) + }) + it("should support merging in hash-keyed param metadata", function () { + const bodyParam = fromJS({ + name: "body", + in: "body" + }) + + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [ + bodyParam + ] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + [`body.body.hash-${bodyParam.hashCode()}`]: { + value: "abc123" + } + } + } + } + } + } + }) + + const result = operationWithMeta(state, "/", "get") + + expect(result.toJS()).toEqual({ + parameters: [ + { + name: "body", + in: "body", + value: "abc123" + } + ] + }) + }) + }) + describe("parameterWithMeta", function() { + it("should support merging in name+in keyed param metadata", function () { + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [ + { + name: "body", + in: "body" + } + ] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + "body.body": { + value: "abc123" + } + } + } + } + } + } + }) + + const result = parameterWithMeta(state, ["/", "get"], "body", "body") + + expect(result.toJS()).toEqual({ + name: "body", + in: "body", + value: "abc123" + }) + }) + it("should give best-effort when encountering hash-keyed param metadata", function () { + const bodyParam = fromJS({ + name: "body", + in: "body" + }) + + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [ + bodyParam + ] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + [`body.body.hash-${bodyParam.hashCode()}`]: { + value: "abc123" + } + } + } + } + } + } + }) + + const result = parameterWithMeta(state, ["/", "get"], "body", "body") + + expect(result.toJS()).toEqual({ + name: "body", + in: "body", + value: "abc123" + }) + }) + + }) + describe("parameterWithMetaByIdentity", function() { + it("should support merging in name+in keyed param metadata", function () { + const bodyParam = fromJS({ + name: "body", + in: "body" + }) + + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [bodyParam] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + "body.body": { + value: "abc123" + } + } + } + } + } + } + }) + + const result = parameterWithMetaByIdentity(state, ["/", "get"], bodyParam) + + expect(result.toJS()).toEqual({ + name: "body", + in: "body", + value: "abc123" + }) + }) + it("should support merging in hash-keyed param metadata", function () { + const bodyParam = fromJS({ + name: "body", + in: "body" + }) + + const state = fromJS({ + json: { + paths: { + "/": { + "get": { + parameters: [ + bodyParam + ] + } + } + } + }, + meta: { + paths: { + "/": { + "get": { + parameters: { + [`body.body.hash-${bodyParam.hashCode()}`]: { + value: "abc123" + } + } + } + } + } + } + }) + + const result = parameterWithMetaByIdentity(state, ["/", "get"], bodyParam) + + expect(result.toJS()).toEqual({ + name: "body", + in: "body", + value: "abc123" + }) + }) + }) })