refactor!: cleanup RequestExt; use extractors instead
#449
Conversation
github-actions
bot
added
C-lib
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to remove convenience methods from the RequestExt trait (db(), cache(), email()) and replaces them with a more idiomatic extractor pattern. This is a breaking change that improves code consistency by having all framework services accessed via extractors rather than a mix of trait methods and extractors.
Changes:
- Removed
db(),cache(), andemail()methods fromRequestExttrait - Added
FromRequestHeadimplementations forDatabase,Cache, andEmailtypes - Updated example code and documentation to use the new extractor pattern
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
examples/send-email/src/main.rs |
Updated to use email_service as a function parameter (extractor) instead of calling request.email() |
cot/tests/extractors.rs |
Added new integration tests demonstrating cache and email extractors |
cot/src/request/extractors.rs |
Implemented FromRequestHead for Database, Cache, and Email types; added unit tests |
cot/src/request.rs |
Removed db(), cache(), and email() methods from RequestExt and removed related tests |
cot/src/project.rs |
Updated documentation example to reflect the removal of request.db() shortcut |
cot/src/db/relations.rs |
Updated documentation example to use Database extractor instead of request.db() |
cot/src/auth/db.rs |
Updated documentation examples to use Database extractor in function signatures |
cot-macros/src/admin.rs |
Updated generated code to use request.context().database() instead of request.db() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cot/src/db/relations.rs
Outdated
| /// ``` | ||
| /// use cot::db::{Auto, ForeignKey, Model, model}; | ||
| /// use cot::db::{Auto, Database, ForeignKey, Model, model}; | ||
| /// use cot::request::{Request, RequestExt}; |
There was a problem hiding this comment.
The import of RequestExt is no longer needed in this documentation example since the code now uses the Database extractor directly instead of request.db(). Consider removing RequestExt from the import statement.
| /// use cot::request::{Request, RequestExt}; |
cot/src/auth/db.rs
Outdated
| /// use cot::common_types::Password; | ||
| /// use cot::db::Database; | ||
| /// use cot::html::Html; | ||
| /// use cot::request::{Request, RequestExt}; |
There was a problem hiding this comment.
The imports of Request and RequestExt are no longer needed in this documentation example since the code now uses the Database extractor directly. Consider removing these unused imports from the import statement.
| /// use cot::request::{Request, RequestExt}; |
cot/src/auth/db.rs
Outdated
| /// use cot::common_types::Password; | ||
| /// use cot::db::Database; | ||
| /// use cot::html::Html; | ||
| /// use cot::request::{Request, RequestExt}; |
There was a problem hiding this comment.
The imports of Request and RequestExt are no longer needed in this documentation example since the code now uses the Database extractor directly. Consider removing these unused imports from the import statement.
| /// use cot::request::{Request, RequestExt}; |
|
Bencher Report| Branch | request-ext-cleanup |
| Testbed | github-ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result microseconds (µs) (Result Δ%) | Upper Boundary microseconds (µs) (Limit %) |
|---|---|---|---|
| empty_router/empty_router | 📈 view plot 🚷 view threshold | 6,170.70 µs(+1.96%)Baseline: 6,051.92 µs | 6,863.83 µs (89.90%) |
| json_api/json_api | 📈 view plot 🚷 view threshold | 1,032.10 µs(+0.96%)Baseline: 1,022.30 µs | 1,140.75 µs (90.48%) |
| nested_routers/nested_routers | 📈 view plot 🚷 view threshold | 946.72 µs(+0.24%)Baseline: 944.41 µs | 1,049.20 µs (90.23%) |
| single_root_route/single_root_route | 📈 view plot 🚷 view threshold | 906.39 µs(+0.26%)Baseline: 904.04 µs | 1,003.48 µs (90.32%) |
| single_root_route_burst/single_root_route_burst | 📈 view plot 🚷 view threshold | 17,116.00 µs(-2.41%)Baseline: 17,537.84 µs | 20,808.28 µs (82.26%) |
seqre
left a comment
seqre
left a comment
There was a problem hiding this comment.
Actually, a change of mind. It should be deprecated and removed in the next release, just like we did it last time.
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
ElijahAhianyo
left a comment
ElijahAhianyo
left a comment
There was a problem hiding this comment.
Ah Nice, this looks like a better approach
No description provided.