Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialize body parameter value #1006 #1012

Closed
wants to merge 4 commits into from
Closed

Serialize body parameter value #1006 #1012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
217d430
Serialize body parameter value #1006
dwilson6 Apr 10, 2017
7c17775
Merge branch 'master' into fix-1006
dwilson6 Apr 12, 2017
1e014a8
Make code more readable. #1006
dwilson6 Apr 14, 2017
f1e9748
Merge branch 'fix-1006' of github.com:dwilson6/swagger-js into fix-1006
dwilson6 Apr 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/execute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,22 @@ export function buildRequest({
const builder = parameterBuilders[parameter.in]
let value

if(parameter.in === 'body' && parameter.schema && parameter.schema.properties) {
//use the parameter value from the parameters object if it is set
if(parameter.name && parameters[parameter.name]) {
value = parameters[parameter.name];
} else if(parameter.in === 'body' && parameter.schema && parameter.schema.properties) {
//if this parameter is in the body and has an object schema, use the whole parameters object
value = parameters
}

value = parameter && parameter.name && parameters[parameter.name]

if (typeof parameter.default !== 'undefined' && typeof value === 'undefined') {
value = parameter.default
}

if (parameter.in === 'body' && typeof value === 'object') {
value = JSON.stringify(value)
}

if (typeof value === 'undefined' && parameter.required && !parameter.allowEmptyValue) {
throw new Error(`Required parameter ${parameter.name} is not provided`)
}
Expand Down
55 changes: 51 additions & 4 deletions test/execute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,13 +529,60 @@ describe('execute', () => {
headers: {
head: 'justTheHead',
},
body: {
body: JSON.stringify({
json: 'rulez'
}
})
})
})
})

it('should use the whole parameters object as the parameter value if parameter is in the body', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webron this is about how the spec works. I'm not sure if what is proposed is valid. Please check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know how to read the test so I don't understand what it does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #1006.

Also, if an api endpoint only has one parameter, what do you think about allowing you to call the operation with the interface operation(value) instead of operation({param_name: value})? If an operation only has one parameter, you could check if the argument passed to the operation is an object with a property matching the parameter name (existing behavior) and if not then just use the passed in argument as the value for the parameter (after validating the type). Are there problems with that approach or does that make it too complicated?

Maybe @fehguy can chime in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't sound like a spec issue, but rather a simplified interface for call execution. It does make sense to me, but would rather hear from @fehguy as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fehguy @webron anything new on this? The code for this feature was already there but was broken

const spec = {
host: 'swagger.io',
paths: {
'/v1/blob/image.png': {
post: {
operationId: 'getBlob',
parameters: [
{
name: 'bodyParam',
in: 'body',
required: true,
schema: {
type: 'object',
properties: {
id: {type: 'integer'},
name: {type: 'string'}
}
}
}
]
}
}
}
}

const req = buildRequest({
spec: spec,
operationId: 'getBlob',
parameters: {
name: 'johny',
id: '123'
}})


expect(req).toEqual({
url: 'http://swagger.io/v1/blob/image.png',
method: 'POST',
credentials: 'same-origin',
body: JSON.stringify({
name: 'johny',
id: '123',
}),
headers: { }
})
})

// Note: this is to handle requestContentType and responseContentType
// although more might end up using it.
it('should pass extras props to buildRequest', () => {
Expand Down Expand Up @@ -1052,10 +1099,10 @@ describe('execute', () => {
url: 'http://swagger.io/v1/blob/image.png?someQuery=foo',
method: 'POST',
credentials: 'same-origin',
body: {
body: JSON.stringify({
name: 'johny',
id: '123',
},
}),
headers: { }
})
})
Expand Down