Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

fix: remove requirement for operationId #69

Merged
merged 1 commit into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 10 additions & 4 deletions lib/backends/swagger-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ class SwaggerClient {
options.qs || options.parameters,
options.pathnameParameters
)

const operationId = options.pathItemObject[options.method.toLowerCase()].operationId
return this.client.execute({
operationId,
parameters
})

const operationReference = operationId ? {
operationId
} : {
pathName: options.pathname,
method: options.method
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://github.com/swagger-api/swagger-js#tryitout-executor
pathName and method can be provided in place of operationId

Possibly even preferred in this case?

// If pathName and method, then those are used instead of operationId. This is useful if you're using this dynamically, as pathName + method are guarenteed to be unique.

Keeping old behaviour when possible seemed like a less intrusive change tho :)


return this.client.execute(Object.assign({}, operationReference, { parameters }))
}
}

Expand Down
28 changes: 27 additions & 1 deletion lib/backends/swagger-client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,33 @@ describe('lib.backends.swagger-client', () => {
}
})).to.be.equal(true)
done()
})
}).catch(done)
})

it('executes operation without operationId', done => {
const swagger = { execute: sinon.spy(() => Promise.resolve('some result')) }
const client = new SwaggerClient({ client: swagger })

client.http({
method: 'GET',
pathname: '/api/v2/foo',
pathItemObject: { get: { } },
body: { a: 1 },
parameters: { b: 2 },
pathnameParameters: { c: 3 }
}).then(() => {
expect(swagger.execute.called).is.equal(true)
expect(swagger.execute.getCall(0).args).to.deep.equal([{
pathName: '/api/v2/foo',
method: 'GET',
parameters: {
a: 1,
b: 2,
c: 3
}
}])
done()
}).catch(done)
})
})
})
1 change: 0 additions & 1 deletion lib/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ class Component {
// "Expose" operations by omitting the leading _ from the method name.
//
Object.keys(endpoint.pathItem)
.filter(key => endpoint.pathItem[key].operationId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I'm not really sure why this filter existed in the first place but it seems fine to remove?

.forEach(method => {
component[method] = component['_' + method]
if (method === 'get') component.getStream = component._getStream
Expand Down
15 changes: 15 additions & 0 deletions lib/loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,21 @@ describe('list.loader', () => {
expect(component.foo('loo').bar.getPath()).to.equal('/foo/loo/bar')
})

it('does not blow up without operationId', () => {
const spec = {
paths: {
'/apis/some/url': {
get: {
}
}
}
}
const component = new Component()
component._addSpec(spec)
expect(component.apis.some.url.get).is.a('function')
expect(component.apis.some.url.getStream).is.a('function')
})

it('tracks parameters correctly', () => {
const spec = {
paths: {
Expand Down