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

add validation schema to shot #71

Merged
merged 3 commits into from Aug 26, 2016
Merged

add validation schema to shot #71

merged 3 commits into from Aug 26, 2016

Conversation

@johnbrett
Copy link
Contributor

johnbrett commented Jul 26, 2016

Fixes #66

dispatchFunc: dispatchFunc,
options: options,
callback: callback
}, Schema);

This comment has been minimized.

Copy link
@hueniverse

hueniverse Jul 26, 2016

Member

{ options, callback } looks nicer

url: internals.url.required(),
headers: Joi.object(),
payload: Joi.any(),
simulate: Joi.object(), // add these

This comment has been minimized.

Copy link
@hueniverse

hueniverse Jul 27, 2016

Member

I agree. Add these... :-)

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Aug 26, 2016

Going to finish it?

@johnbrett

This comment has been minimized.

Copy link
Contributor Author

johnbrett commented Aug 26, 2016

Cheers for the reminder @hueniverse - added the simulate options.

@hueniverse hueniverse merged commit 4d63193 into hapijs:master Aug 26, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hueniverse hueniverse added the feature label Aug 26, 2016
@hueniverse hueniverse added this to the 3.2.1 milestone Aug 26, 2016
@hueniverse hueniverse self-assigned this Aug 26, 2016
@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Aug 26, 2016

Hmm. Now that I think about it, this will make things very slow when inject() is used in live code (e.g. not in tests). Will need a way to bypass it. I'll add something tomorrow.

@johnbrett

This comment has been minimized.

Copy link
Contributor Author

johnbrett commented Aug 26, 2016

Yeah true, kind of the opposite direction to hapijs/hapi#2751 (Optimize server.inject() for runtime usage).

Could only run validate when NODE_ENV !== 'production' or something along those lines?

@antony

This comment has been minimized.

Copy link

antony commented Aug 26, 2016

NODE_ENV !== 'production' won't work. You'd want this validation disabled in perf testing environments too, and we have a different NODE_ENV variable for every region we are deployed in :)

Just needs a config variable or some sort of server.app attribute or similar.

@mtharrison

This comment has been minimized.

Copy link
Member

mtharrison commented Aug 26, 2016

Agreed - I wouldn't ever want nes requests to pass through this validation step. An inject() option should do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.