pistacheio / pistache

A high-performance REST toolkit written in C++
https://pistacheio.github.io/pistache/
Apache License 2.0
3.12k stars 688 forks source link

feat: add Request::getBestAcceptEncoding() function #1165

Closed Tachi107 closed 8 months ago

Tachi107 commented 8 months ago

The new getBestAcceptEncoding() function makes using the new compression support introduced in commit b0b7d3076dcdba48935cfe14049b76809d0cd256 way easier, as it reports to the caller the most sensible content encoding to apply to the response, falling back to the Identity encoding (i.e. no actual compression) in case no encoding is supported by both the server and the client.

Fixes https://github.com/pistacheio/pistache/issues/1148

codecov[bot] commented 8 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (fe5639a) 77.92% compared to head (d1b0718) 78.07%.

:exclamation: Current head d1b0718 differs from pull request most recent head 3d40fd2. Consider uploading reports for the commit 3d40fd2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1165 +/- ## ========================================== + Coverage 77.92% 78.07% +0.15% ========================================== Files 53 53 Lines 6990 7062 +72 ========================================== + Hits 5447 5514 +67 - Misses 1543 1548 +5 ``` | [Files](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio) | Coverage Δ | | |---|---|---| | [include/pistache/http.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwLmg=) | `86.48% <ø> (ø)` | | | [include/pistache/http\_header.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlci5o) | `91.56% <100.00%> (-0.25%)` | :arrow_down: | | [include/pistache/http\_headers.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlcnMuaA==) | `78.57% <ø> (-0.92%)` | :arrow_down: | | [src/common/mime.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9taW1lLmNj) | `88.88% <100.00%> (-1.17%)` | :arrow_down: | | [src/common/http\_headers.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlcnMuY2M=) | `86.41% <96.55%> (+5.96%)` | :arrow_up: | | [src/common/http.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwLmNj) | `75.46% <83.33%> (+0.10%)` | :arrow_up: | | [src/common/http\_header.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlci5jYw==) | `91.74% <92.59%> (+0.07%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/pistacheio/pistache/pull/1165/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Tachi107 commented 8 months ago

Added some more tests and fixed a corner case discovered with the newly added tests. Should be ready now :)

kiplingw commented 8 months ago

@Tachi107, if your PR passes in CI and @dennisjenkins75 has had a chance to review, I'm totally fine with it. Only thing I'd suggest changing is to bump the patch version.

Tachi107 commented 8 months ago

Only thing I'd suggest changing is to bump the patch version.

Done

Edit: abidiff failures expected, adding new stuff changes the ABI

Tachi107 commented 8 months ago

Debian testing CI runs are failing the Coverage step because of Debian bug https://bugs.debian.org/1053252, it's unrelated to this patch set.

Tachi107 commented 8 months ago

Thanks for the review and merge :)