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.
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
Expand fs-ts in Prism's router #402
Changes from all commits
4183898
6f1ae0f
42e4d22
e45b0a8
7ca5429
7b72eaf
5742996
de8ba3a
7c802d9
347b589
4accf3b
7b728d2
22600a5
d972f3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| import { DiagnosticSeverity } from '@stoplight/types'; | ||
| import { toError } from 'fp-ts/lib/Either'; | ||
| import { fromEither, left2v, right2v, tryCatch } from 'fp-ts/lib/TaskEither'; | ||
| import { configMergerFactory, PartialPrismConfig, PrismConfig } from '.'; | ||
| import { IPrism, IPrismComponents, IPrismConfig, IPrismDiagnostic, PickRequired, ProblemJsonError } from './types'; | ||
|
|
||
|
|
@@ -39,93 +41,114 @@ export function factory<Resource, Input, Output, Config, LoadOpts>( | |
| const configObj: Config | undefined = configMerger(input); | ||
| const inputValidations: IPrismDiagnostic[] = []; | ||
|
|
||
| // find the correct resource | ||
| let resource: Resource | undefined; | ||
| if (components.router) { | ||
| try { | ||
| resource = components.router.route({ resources, input, config: configObj }, defaultComponents.router); | ||
| } catch (error) { | ||
| // rethrow error we if we're attempting to mock | ||
| if ((configObj as IPrismConfig).mock) { | ||
| throw error; | ||
| } | ||
| const { message, name, status } = error as ProblemJsonError; | ||
| // otherwise let's just stack it on the inputValidations | ||
| // when someone simply wants to hit an URL, don't block them | ||
| inputValidations.push({ | ||
| message, | ||
| source: name, | ||
| code: status, | ||
| severity: DiagnosticSeverity.Warning, | ||
| }); | ||
| } | ||
| } | ||
| return components.router | ||
| .route({ resources, input, config: configObj }, defaultComponents.router) | ||
| .fold( | ||
| error => { | ||
| // rethrow error we if we're attempting to mock | ||
| if ((configObj as IPrismConfig).mock) { | ||
| return left2v(error); | ||
| } | ||
|
|
||
| // validate input | ||
| if (resource && components.validator && components.validator.validateInput) { | ||
| inputValidations.push( | ||
| ...(await components.validator.validateInput( | ||
| { | ||
| resource, | ||
| input, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.validator, | ||
| )), | ||
| ); | ||
| } | ||
| const { message, name, status } = error as ProblemJsonError; | ||
| // otherwise let's just stack it on the inputValidations | ||
| // when someone simply wants to hit an URL, don't block them | ||
| inputValidations.push({ | ||
| message, | ||
| source: name, | ||
| code: status, | ||
| severity: DiagnosticSeverity.Warning, | ||
| }); | ||
|
|
||
| // build output | ||
| let output: Output | undefined; | ||
| if (resource && components.mocker && (configObj as IPrismConfig).mock) { | ||
| // generate the response | ||
| output = components.mocker | ||
| .mock( | ||
| { | ||
| resource, | ||
| input: { validations: { input: inputValidations }, data: input }, | ||
| config: configObj, | ||
| return right2v<Error, Resource | undefined>(undefined); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's actually a good point — it has been fixed in 2.x but I didn't want to deal with breaking changes right now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll get less PR from random people. Entry threshold is set high ;) |
||
| }, | ||
| defaultComponents.mocker, | ||
| value => right2v(value), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a problem finding documentation for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is deprecated in 2.x — Prism is currently on 1.9. I'll migrate it soon though. |
||
| ) | ||
| .run(components.logger.child({ name: 'NEGOTIATOR' })) | ||
| .fold( | ||
| e => { | ||
| throw e; | ||
| }, | ||
| r => r, | ||
| ); | ||
| } else if (components.forwarder) { | ||
| // forward request and set output from response | ||
| output = await components.forwarder.forward( | ||
| { | ||
| resource, | ||
| input: { validations: { input: inputValidations }, data: input }, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.forwarder, | ||
| ); | ||
| } | ||
| .chain(resource => { | ||
| // validate input | ||
| if (resource && components.validator && components.validator.validateInput) { | ||
| inputValidations.push( | ||
| ...components.validator.validateInput( | ||
| { | ||
| resource, | ||
| input, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.validator, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| if (resource && components.mocker && (configObj as IPrismConfig).mock) { | ||
| // generate the response | ||
| return fromEither( | ||
| components.mocker | ||
| .mock( | ||
| { | ||
| resource, | ||
| input: { validations: { input: inputValidations }, data: input }, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.mocker, | ||
| ) | ||
| .run(components.logger.child({ name: 'NEGOTIATOR' })), | ||
| ).map(output => ({ output, resource })); | ||
| } else if (components.forwarder) { | ||
| // forward request and set output from response | ||
| return components.forwarder | ||
| .fforward( | ||
| { | ||
| resource, | ||
| input: { validations: { input: inputValidations }, data: input }, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.forwarder, | ||
| ) | ||
| .map(output => ({ output, resource })); | ||
| } | ||
|
|
||
| // validate output | ||
| let outputValidations: IPrismDiagnostic[] = []; | ||
| if (resource && components.validator && components.validator.validateOutput) { | ||
| outputValidations = await components.validator.validateOutput( | ||
| { | ||
| resource, | ||
| output, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.validator, | ||
| ); | ||
| return left2v(new Error('Nor forwarder nor mocker has been selected. Something is wrong')); | ||
| }) | ||
| .map(({ output, resource }) => { | ||
| let outputValidations: IPrismDiagnostic[] = []; | ||
| if (resource && components.validator && components.validator.validateOutput) { | ||
| outputValidations = components.validator.validateOutput( | ||
| { | ||
| resource, | ||
| output, | ||
| config: configObj, | ||
| }, | ||
| defaultComponents.validator, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| input, | ||
| output, | ||
| validations: { | ||
| input: inputValidations, | ||
| output: outputValidations, | ||
| }, | ||
| }; | ||
| }) | ||
| .run() | ||
| .then(v => | ||
| v.fold( | ||
| e => { | ||
| throw e; | ||
| }, | ||
| o => o, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| input, | ||
| output, | ||
| output: undefined, | ||
| validations: { | ||
| input: inputValidations, | ||
| output: outputValidations, | ||
| input: [], | ||
| output: [], | ||
| }, | ||
| }; | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,14 +188,9 @@ describe.each([['petstore.oas2.json'], ['petstore.oas3.json']])('server %s', fil | |
| checkErrorPayloadShape(response.payload); | ||
| }); | ||
|
|
||
| it.skip('should automagically provide the parameters when not provided in the query string and a default is defined', async () => { | ||
| const response = await server.fastify.inject({ | ||
| method: 'GET', | ||
| url: '/pets/findByStatus', | ||
| }); | ||
|
|
||
| expect(response.statusCode).toBe(200); | ||
| }); | ||
| test.todo( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Such a decent test :( why remove it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was skipped — I'm fine putting it back but it's not getting executed anyway. I'd rather see a todo that says "Hey write this" instead of a skip that you do not easily see |
||
| 'should automagically provide the parameters when not provided in the query string and a default is defined', | ||
| ); | ||
|
|
||
| it('should support multiple param values', async () => { | ||
| const response = await server.fastify.inject({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,18 +201,8 @@ describe('Http Client .process', () => { | |
| }); | ||
| }); | ||
|
|
||
| describe('GET /pet without an optional body parameter', () => { | ||
| // TODO will be fixed by https://stoplightio.atlassian.net/browse/SO-260 | ||
| xit('returns 200 response', async () => { | ||
| const response = await prism.process({ | ||
| method: 'get', | ||
| url: { path: '/pet' }, | ||
| }); | ||
|
|
||
| expect(response.output).toBeDefined(); | ||
| expect(response.output!.statusCode).toEqual(200); | ||
| }); | ||
| }); | ||
| // TODO will be fixed by https://stoplightio.atlassian.net/browse/SO-260 | ||
| test.todo('GET /pet without an optional body parameter'); | ||
|
|
||
| describe('when processing GET /pet/findByStatus', () => { | ||
| it('with valid query params returns generated body', async () => { | ||
|
|
@@ -269,24 +259,7 @@ describe('Http Client .process', () => { | |
| }); | ||
|
|
||
| // TODO: will be fixed by https://stoplightio.atlassian.net/browse/SO-259 | ||
| xit('with invalid body returns validation errors', () => { | ||
| return expect( | ||
| prism.process({ | ||
| method: 'get', | ||
| url: { | ||
| path: '/pet/findByStatus', | ||
| query: { | ||
| status: ['available'], | ||
| }, | ||
| }, | ||
| body: { | ||
| id: 'should not be a string', | ||
| status: 'should be one of "placed", "approved", "delivered"', | ||
| complete: 'should be a boolean', | ||
| }, | ||
| }), | ||
| ).rejects.toThrowError(ProblemJsonError.fromTemplate(UNPROCESSABLE_ENTITY)); | ||
| }); | ||
| test.todo('with invalid body returns validation errors'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 🔥 🔥 You're removing my lovely tests :( 🔥 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was skipped — I'm fine putting it back but it's not getting executed anyway. I'd rather see a todo that says "Hey write this" instead of a skip that you do not easily see |
||
| }); | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing things to array which is defined outside function chains feels not right. How about using validations? (https://dev.to/gcanti/getting-started-with-fp-ts-either-vs-validation-5eja - Validation section in the middle)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! That's exactly what I wanted to do. However I'm proceeding with baby steps here as — you've probably noticed — it's hard to digest all in one place; moreover we're close to the release and I didn't want to change that many things at the time.
…but yes it's exactly the direction I want to take!