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.

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

Jump to bottom

Validation refactor pass 2 #862

Merged
merged 32 commits into from Dec 2, 2019
Merged

Validation refactor pass 2 #862

merged 32 commits into from Dec 2, 2019

Conversation

XVincentX
Copy link
Contributor

@XVincentX XVincentX commented Nov 29, 2019 •
edited

This PR comes out because of an attempt of using These in Prism, resulting in some inconsistencies that were not making the thing possible:

  • validateBody is not doing the fromEither and fromOption conversion anymore
  • Correctly use NonEmptyArray where appropriate
  • validateAgainstSchema function now returns None when no validation errors instead of an empty array — making the things a little bit weird.

@XVincentX XVincentX marked this pull request as ready for review November 29, 2019 16:43
@XVincentX XVincentX force-pushed the ref/validation/parsed branch 8 times, most recently from 8cefd83 to 9c1a0ff Compare December 2, 2019 09:16
@XVincentX
refactor: put output function outside
38a4e7e
@XVincentX
refactor: input is not required
3209ec8
@XVincentX
refactor: return a non empty array when appropriate
e94d38f
@XVincentX
feat: add additional return type
30cc476
@XVincentX
refactor: use nonemptyarray and reorder the flow
d33e6a9
@XVincentX
use Dictionary<string> once
91a3185
@XVincentX
subdivide parsing and condition checking
f7accbf
@XVincentX
refactor: use fold to explicitly go through the flows
eb73dd7
@XVincentX
refactor: fix all the tests whenever is required
e8f261c
@XVincentX
refactor: remove useless check
dd7c388
@XVincentX
refactor: sometimes ifs are just ok?
86bb1e4
@XVincentX
refactor: undo type change
5482de3
@XVincentX
refactor: prefer fold
4eb10d2
@XVincentX
refactor: remove function
59ed906
@XVincentX
refactor: make the function more clear
398e70f
@XVincentX
refactor: map only once
e0d3c1f
@XVincentX
refactor: make the flow even cleaner
86bcdd9
@XVincentX
refactor: parseHeaders takes the minimal stuff
b3c3122
@XVincentX
refactor: return immediately
17f6030
@XVincentX
refactor: separate functions for better clarity
f6a48ab
karol-maciaszek
karol-maciaszek suggested changes Dec 2, 2019
View reviewed changes
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

It good to see another chunk of code to be on the functional side of the river. Added few minor comments and questions.

packages/http/src/mocker/callback/callbacks.ts Outdated Show resolved Hide resolved
packages/http/src/mocker/negotiator/NegotiatorHelpers.ts Show resolved Hide resolved
packages/http/src/validator/validators/utils.ts Outdated Show resolved Hide resolved
packages/http/src/validator/validators/body.ts Outdated Show resolved Hide resolved
packages/http/src/validator/validators/body.ts Outdated Show resolved Hide resolved
packages/http/src/validator/validators/body.ts Outdated Show resolved Hide resolved
karol-maciaszek
karol-maciaszek approved these changes Dec 2, 2019
View reviewed changes
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

Good job!

@XVincentX XVincentX merged commit 824c6c5 into master Dec 2, 2019
@XVincentX XVincentX deleted the ref/validation/parsed branch December 2, 2019 16:10
@XVincentX XVincentX mentioned this pull request Apr 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karol-maciaszek karol-maciaszek karol-maciaszek approved these changes

Assignees

@karol-maciaszek karol-maciaszek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@XVincentX @karol-maciaszek