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

Made TypeSafeMiddleware initialize first in codable routes #1350

Merged
merged 4 commits into from Dec 18, 2018

Conversation

Projects
None yet
2 participants
@Andrew-Lees11
Copy link
Contributor

Andrew-Lees11 commented Oct 22, 2018

Description

This PR changes the TypeSafeMiddleware handlers so that the TypeSafeMiddleware is initialized before, Query parameters, Identifiers and Codable objects.

Motivation and Context

This change was requested for two reasons:

  • So that you can have a TypeSafeMiddleware that logs Query parameters even when the query fails to be decoded
  • So that you can have a TypeSafeMiddleware that altered the request before it is decoded into a Codable object (E.g. Changing the request decoder that will be used)

How Has This Been Tested?

The existing TypeSafeMiddleware tests have been run and are all still passing.

@Andrew-Lees11 Andrew-Lees11 requested a review from ianpartridge Oct 22, 2018

@Andrew-Lees11 Andrew-Lees11 requested review from djones6 and removed request for ianpartridge Nov 21, 2018

@djones6

This comment has been minimized.

Copy link
Member

djones6 commented Dec 13, 2018

@Andrew-Lees11 can you add tests for the two example cases you mentioned in your opening comment?

@Andrew-Lees11

This comment has been minimized.

Copy link
Contributor

Andrew-Lees11 commented Dec 13, 2018

@djones6
I have added a test case for logging the query parameters using a typeSafeMiddleware even if they are incorrect. This would have failed before but is now possible since the middleware is run before the query is decoded. With changing decoder, since we are no longer making them public this is no longer a use case.

@djones6 djones6 merged commit e64cd66 into master Dec 18, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@djones6 djones6 deleted the TSMiddlewareFirst branch Dec 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment