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

feat!: use extractor pattern for request handlers #253

Merged
merged 6 commits into from
Mar 25, 2025
Merged

Conversation

m4tx
Copy link
Member

@m4tx m4tx commented Mar 22, 2025

This is a massive change and the transition probably isn't even fully done yet. This should be a good starting point, though.

The remaining work includes:

  • Removing more of the RequestExt trait
  • Creating IntoResponse trait so that the consumers can return other types than cot::Result<Response>
  • More work on maintaining consistency in the API

Note that this is a (quite big) breaking change, mostly because it removes a lot of methods from the RequestExt trait.

This change is needed to implement automatic OpenAPI spec generation in #159.

@m4tx m4tx requested a review from seqre March 22, 2025 15:07
@github-actions github-actions bot added C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI) C-lib Crate: cot (main library crate) labels Mar 22, 2025
@m4tx m4tx force-pushed the new-request-api branch 2 times, most recently from 8e2c734 to ebdab62 Compare March 22, 2025 15:14
Copy link

codecov bot commented Mar 22, 2025

Codecov Report

Attention: Patch coverage is 83.22521% with 181 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cot/src/admin.rs 0.00% 72 Missing ⚠️
cot/src/handler.rs 52.38% 13 Missing and 7 partials ⚠️
cot/src/auth.rs 91.97% 4 Missing and 11 partials ⚠️
cot/src/db.rs 28.57% 15 Missing ⚠️
cot/src/request/extractors.rs 94.20% 2 Missing and 10 partials ⚠️
cot/src/middleware.rs 91.91% 7 Missing and 4 partials ⚠️
cot/src/project.rs 83.87% 10 Missing ⚠️
cot/src/request.rs 96.55% 6 Missing and 1 partial ⚠️
cot/src/router.rs 80.64% 6 Missing ⚠️
cot/src/test.rs 87.50% 4 Missing ⚠️
... and 5 more
Flag Coverage Δ
rust 81.65% <83.22%> (+0.54%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cot-macros/src/lib.rs 95.91% <ø> (ø)
cot/src/config.rs 86.90% <100.00%> (-0.74%) ⬇️
cot/src/error.rs 88.75% <ø> (ø)
cot/src/request/path_params_deserializer.rs 64.46% <ø> (ø)
cot/src/static_files.rs 93.80% <100.00%> (+0.07%) ⬆️
cot/src/cli.rs 80.74% <50.00%> (-0.41%) ⬇️
cot/src/response.rs 97.26% <85.71%> (+0.24%) ⬆️
cot/src/auth/db.rs 77.60% <60.00%> (+0.12%) ⬆️
cot/src/form.rs 67.00% <95.34%> (+19.54%) ⬆️
cot/src/session.rs 82.35% <82.35%> (ø)
... and 10 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m4tx m4tx force-pushed the new-request-api branch 2 times, most recently from eab4e0e to 05c3be4 Compare March 22, 2025 15:40
This is a massive change and the transition probably isn't even fully
done yet. This should be a good starting point, though.

The remaining work includes:

* Removing more of the `RequestExt` trait
* Creating `IntoResponse` trait so that the consumers can return other
  types than `cot::Result<Response>`
* More work on maintaining consistency in the API

Note that this is a (quite big) breaking change, mostly because it
removes a lot of methods from the `RequestExt` trait.

This change is needed to implement automatic OpenAPI spec generation
in #159.
@m4tx m4tx force-pushed the new-request-api branch from 05c3be4 to 3c4f110 Compare March 22, 2025 16:09
Copy link
Member

@seqre seqre left a comment

Choose a reason for hiding this comment

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

At the very end, you should go over the generated tests and remove the pointless comments.

@m4tx m4tx requested a review from seqre March 24, 2025 20:33
@github-actions github-actions bot added the C-macros Crate: cot-macros label Mar 24, 2025
Copy link
Member

@seqre seqre left a comment

Choose a reason for hiding this comment

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

Great work! Hit the Codecov's limit of 81.1% and I'll gladly approve

@m4tx m4tx requested a review from seqre March 25, 2025 00:09
Copy link
Member

@seqre seqre left a comment

Choose a reason for hiding this comment

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

👌🎉

@m4tx m4tx merged commit 525c129 into master Mar 25, 2025
30 checks passed
@m4tx m4tx deleted the new-request-api branch March 25, 2025 09:25
@cotbot cotbot bot mentioned this pull request Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI) C-lib Crate: cot (main library crate) C-macros Crate: cot-macros
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants