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

Port cookie extractors to use state to extract keys #1250

Merged
merged 1 commit into from
Aug 12, 2022

Conversation

davidpdrsn
Copy link
Member

This PR builds on #1155 by porting SignedCookieJar and PrivateCookieJar to extract the keys via state rather than loosely typed extensions.

Comment on lines +292 to +296
impl From<AppState> for Key {
fn from(state: AppState) -> Key {
state.key
}
}
Copy link
Member

Choose a reason for hiding this comment

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

So, what does the compiler error look like when this impl is missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its pretty good actually

error[E0277]: the trait bound `cookie::Key: From<AppState>` is not satisfied
   --> axum-extra/src/extract/cookie/mod.rs:236:36
    |
236 |                     .route("/set", get(set_cookie))
    |                                    ^^^ the trait `From<AppState>` is not implemented for `cookie::Key`
...
281 |     cookie_test!(signed_cookies, SignedCookieJar);
    |     --------------------------------------------- in this macro invocation
    |
    = help: the trait `From<CustomKey>` is implemented for `cookie::Key`
    = note: required because of the requirements on the impl of `Into<cookie::Key>` for `AppState`
note: required because of the requirements on the impl of `axum::extract::FromRequest<hyper::Body, AppState>` for `signed::SignedCookieJar`

@davidpdrsn davidpdrsn merged commit c055d0b into router-state-extractor Aug 12, 2022
@davidpdrsn davidpdrsn deleted the router-state-extractor--cookies branch August 12, 2022 10:08
davidpdrsn added a commit that referenced this pull request Aug 17, 2022
* begin threading the state through

* Pass state to extractors

* make state extractor work

* make sure nesting with different states work

* impl Service for MethodRouter<()>

* Fix some of axum-macro's tests

* Implement more traits for `State`

* Update examples to use `State`

* consistent naming of request body param

* swap type params

* Default the state param to ()

* fix docs references

* Docs and handler state refactoring

* docs clean ups

* more consistent naming

* when does MethodRouter implement Service?

* add missing docs

* use `Router`'s default state type param

* changelog

* don't use default type param for FromRequest and RequestParts

probably safer for library authors so you don't accidentally forget

* fix examples

* minor docs tweaks

* clarify how to convert handlers into services

* group methods in one impl block

* make sure merged `MethodRouter`s can access state

* fix docs link

* test merge with same state type

* Document how to access state from middleware

* Port cookie extractors to use state to extract keys (#1250)

* Updates ECOSYSTEM with a new sample project (#1252)

* Avoid unhelpful compiler suggestion (#1251)

* fix docs typo

* document how library authors should access state

* Add `RequestParts::with_state`

* fix example

* apply suggestions from review

* add relevant changes to axum-extra and axum-core changelogs

* Add `route_service_with_tsr`

* fix trybuild expectations

* make sure `SpaRouter` works with routers that have state

* Change order of type params on FromRequest and RequestParts

* reverse order of `RequestParts::with_state` args to match type params

* Add `FromRef` trait (#1268)

* Add `FromRef` trait

* Remove unnecessary type params

* format

* fix docs link

* format examples

* Avoid unnecessary `MethodRouter`

* apply suggestions from review

Co-authored-by: Dani Pardo <dani.pardo@inmensys.com>
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants