Skip to content
GitHub no longer supports this web browser. Learn more about the browsers we support.
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

Fix nested cookies #400

Open
wants to merge 6 commits into
base: master
from
Open

Fix nested cookies #400

wants to merge 6 commits into from

Conversation

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Feb 1, 2020

Fixes #396 by ensuring the cookies middleware is only created when we actually call .listen and not when nested.

This also adds a into_mock method so that the cookies middleware can still be constructed when running tests. This seemed like a good setup to start improving our test workflow, by at least starting to remove some of the boilerplate involved now.

Thanks!

yoshuawuyts added 3 commits Feb 1, 2020
Fixes cookies middleware issues on nesting.

Also ran cargo fmt.
@yoshuawuyts yoshuawuyts requested a review from tirr-c Feb 1, 2020
yoshuawuyts added 3 commits Feb 1, 2020
@isgj

This comment has been minimized.

Copy link
Contributor

isgj commented Feb 1, 2020

Just throwing it out there. Should you provide a function to listen without any default middleware?

/// # Middleware
///
/// This method does not enable default middleware. See `into_mock` for
/// converting Tide into a server that can be used for testing purposes.
pub fn into_http_service(self) -> Service<State> {

This comment has been minimized.

Copy link
@tirr-c

tirr-c Feb 2, 2020

Collaborator

How about leaving this as is, and provide a new method that doesn't apply any default middleware? One rarely calls .into_http_service() directly for nesting since Route::nest handles it instead, so I think it should be okay?

/// Convert the server into a mock server.
///
/// Unlike `into_http_service` this method enables default middleware.
///
/// # Stability
///
/// This method will be expanded in the future to allow mocking requests
/// in a more direct manner.
pub fn into_mock(mut self) -> io::Result<http_service_mock::TestBackend<Service<State>>> {
self.enable_default_middleware();
let server = http_service_mock::make_server(self.into_http_service())?;
Ok(server)
}
Comment on lines +333 to +345

This comment has been minimized.

Copy link
@tirr-c

tirr-c Feb 2, 2020

Collaborator

By providing separate method for nesting, we don't need to provide a "special" wrapper method like this.

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