Merge pull request #1115 from owenconti/bug/swagger-ui-3511

Change how parameter values are resolved

Author: shockey

src/execute.js

@@ -17,6 +17,10 @@ const OperationNotFoundError = createError('OperationNotFoundError', function (m
   Object.assign(this, extra || {})
 })
 
+const findParametersWithName = (name, parameters) => {
+  return parameters.filter(p => p.name === name)
+}
+
 // For stubbing in tests
 export const self = {
   buildRequest
@@ -118,34 +122,47 @@ export function buildRequest({
     req.headers.accept = responseContentType
   }
 
-  // Add values to request
-  arrayOrEmpty(operation.parameters) // operation parameters
+  const combinedParameters = []
+    .concat(arrayOrEmpty(operation.parameters)) // operation parameters
     .concat(arrayOrEmpty(path.parameters)) // path parameters
-    .forEach((parameter) => {
-      const builder = parameterBuilders[parameter.in]
-      let value
 
-      // REVIEW: OAS3: have any key names or parameter shapes changed?
-      // Any new features that need to be plugged in here?
+  // REVIEW: OAS3: have any key names or parameter shapes changed?
+  // Any new features that need to be plugged in here?
 
-      if (parameter.in === 'body' && parameter.schema && parameter.schema.properties) {
-        value = parameters
-      }
 
-      value = parameter && parameter.name && parameters[parameter.name]
+  // Add values to request
+  combinedParameters.forEach((parameter) => {
+    const builder = parameterBuilders[parameter.in]
+    let value
 
-      if (typeof parameter.default !== 'undefined' && typeof value === 'undefined') {
-        value = parameter.default
-      }
+    if (parameter.in === 'body' && parameter.schema && parameter.schema.properties) {
+      value = parameters
+    }
 
-      if (typeof value === 'undefined' && parameter.required && !parameter.allowEmptyValue) {
-        throw new Error(`Required parameter ${parameter.name} is not provided`)
-      }
+    value = parameter && parameter.name && parameters[parameter.name]
 
-      if (builder) {
-        builder({req, parameter, value, operation, spec, specIsOAS3})
-      }
-    })
+    if (typeof value === 'undefined') {
+        // check for `name-in` formatted key
+      value = parameter && parameter.name && parameters[`${parameter.in}.${parameter.name}`]
+    }
+    else if (findParametersWithName(parameter.name, combinedParameters).length > 1) {
+      // value came from `parameters[parameter.name]`
+      // check to see if this is an ambiguous parameter
+      console.warn(`Parameter '${parameter.name}' is ambiguous because the defined spec has more than one parameter with the name: '${parameter.name}' and the passed-in parameter values did not define an 'in' value.`)
+    }
+
+    if (typeof parameter.default !== 'undefined' && typeof value === 'undefined') {
+      value = parameter.default
+    }
+
+    if (typeof value === 'undefined' && parameter.required && !parameter.allowEmptyValue) {
+      throw new Error(`Required parameter ${parameter.name} is not provided`)
+    }
+
+    if (builder) {
+      builder({req, parameter, value, operation, spec})
+    }
+  })
 
   const requestBodyDef = operation.requestBody || {}
   const requestBodyMediaTypes = Object.keys(requestBodyDef.content || {})
@@ -198,7 +215,6 @@ export function buildRequest({
     }
   }
 
-
   // Add securities, which are applicable
   // REVIEW: OAS3: what changed in securities?
   req = applySecurities({request: req, securities, operation, spec})

test/execute.js

@@ -1348,6 +1348,75 @@ describe('execute', () => {
           headers: { }
         })
       })
+
+      it('should fall back to `name-in` format when a parameter cannot be found', function () {
+        // Given
+        const spec = {
+          host: 'swagger.io',
+          basePath: '/v1',
+          paths: {
+            '/one': {
+              get: {
+                operationId: 'getMe',
+                parameters: [{
+                  name: 'name',
+                  in: 'query',
+                  type: 'string'
+                }]
+              }
+            }
+          }
+        }
+
+        // When
+        const req = buildRequest({spec, operationId: 'getMe', parameters: {'query.name': 'john'}})
+
+        // Then
+        expect(req).toEqual({
+          url: 'http://swagger.io/v1/one?name=john',
+          method: 'GET',
+          credentials: 'same-origin',
+          headers: { }
+        })
+      })
+
+      it('should set all parameter options when given an ambiguous parameter value', function () {
+        // Given
+        const spec = {
+          host: 'swagger.io',
+          basePath: '/v1',
+          paths: {
+            '/one': {
+              get: {
+                operationId: 'getMe',
+                parameters: [{
+                  name: 'name',
+                  in: 'query',
+                  type: 'string'
+                }, {
+                  name: 'name',
+                  in: 'formData',
+                  type: 'string'
+                }]
+              }
+            }
+          }
+        }
+
+        // When
+        const req = buildRequest({spec, operationId: 'getMe', parameters: {name: 'john'}})
+
+        // Then
+        expect(req).toEqual({
+          url: 'http://swagger.io/v1/one?name=john',
+          method: 'GET',
+          credentials: 'same-origin',
+          headers: {
+            'Content-Type': 'application/x-www-form-urlencoded'
+          },
+          body: 'name=john'
+        })
+      })
     })
 
     describe('body', function () {