Quality / QualityItem improvements (actix#2486)

.cargo/config.toml

@@ -5,8 +5,10 @@ lint-all = "clippy --workspace --all-features --tests --examples --bins -- -Dcli
 # lib checking
 ci-check-min = "hack --workspace check --no-default-features"
 ci-check-default = "hack --workspace check"
+ci-check-default-tests = "check --workspace --tests"
 ci-check-all-feature-powerset="hack --workspace --feature-powerset --skip=__compress,io-uring check"
 ci-check-all-feature-powerset-linux="hack --workspace --feature-powerset --skip=__compress check"
 
 # testing
+ci-doctest-default = "test --workspace --doc --no-fail-fast -- --nocapture"
 ci-doctest = "test --workspace --all-features --doc --no-fail-fast -- --nocapture"

actix-http/CHANGES.md

@@ -11,6 +11,9 @@
 * `impl Clone for ws::HandshakeError`. [#2468]
 * `#[must_use]` for `ws::Codec` to prevent subtle bugs. [#1920]
 * `impl Default ` for `ws::Codec`. [#1920]
+* `header::QualityItem::{max, min}`. [#2486]
+* `header::Quality::{MAX, MIN}`. [#2486]
+* `impl Display` for `header::Quality`. [#2486]
 
 ### Changed
 * Rename `body::BoxBody::{from_body => new}`. [#2468]
@@ -27,10 +30,13 @@
 * Remove unnecessary `MessageBody` bound on types passed to `body::AnyBody::new`. [#2468]
 * Move `body::AnyBody` to `awc`. Replaced with `EitherBody` and `BoxBody`. [#2468]
 * `impl Copy` for `ws::Codec`. [#1920]
+* `header::qitem` helper. Replaced with `header::QualityItem::max` [#2486]
+* `impl TryFrom<u16>` for `header::Quality` [#2486]
 
 [#2483]: https://github.com/actix/actix-web/pull/2483
 [#2468]: https://github.com/actix/actix-web/pull/2468
 [#1920]: https://github.com/actix/actix-web/pull/1920
+[#2486]: https://github.com/actix/actix-web/pull/2486
 
 
 ## 3.0.0-beta.14 - 2021-11-30

actix-http/Cargo.toml

@@ -112,3 +112,7 @@ harness = false
 [[bench]]
 name = "uninit-headers"
 harness = false
+
+[[bench]]
+name = "quality-value"
+harness = false

actix-http/benches/quality-value.rs

@@ -0,0 +1,90 @@
+use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
+
+const CODES: &[u16] = &[0, 1000, 201, 800, 550];
+
+fn bench_quality_display_impls(c: &mut Criterion) {
+    let mut group = c.benchmark_group("quality value display impls");
+
+    for i in CODES.iter() {
+        group.bench_with_input(BenchmarkId::new("New (fast?)", i), i, |b, &i| {
+            b.iter(|| _new::Quality(i).to_string())
+        });
+
+        group.bench_with_input(BenchmarkId::new("Naive", i), i, |b, &i| {
+            b.iter(|| _naive::Quality(i).to_string())
+        });
+    }
+
+    group.finish();
+}
+
+criterion_group!(benches, bench_quality_display_impls);
+criterion_main!(benches);
+
+mod _new {
+    use std::fmt;
+
+    pub struct Quality(pub(crate) u16);
+
+    impl fmt::Display for Quality {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            match self.0 {
+                0 => f.write_str("0"),
+                1000 => f.write_str("1"),
+
+                // some number in the range 1–999
+                x => {
+                    f.write_str("0.")?;
+
+                    // this implementation avoids string allocation otherwise required
+                    // for `.trim_end_matches('0')`
+
+                    if x < 10 {
+                        f.write_str("00")?;
+                        // 0 is handled so it's not possible to have a trailing 0, we can just return
+                        itoa::fmt(f, x)
+                    } else if x < 100 {
+                        f.write_str("0")?;
+                        if x % 10 == 0 {
+                            // trailing 0, divide by 10 and write
+                            itoa::fmt(f, x / 10)
+                        } else {
+                            itoa::fmt(f, x)
+                        }
+                    } else {
+                        // x is in range 101–999
+
+                        if x % 100 == 0 {
+                            // two trailing 0s, divide by 100 and write
+                            itoa::fmt(f, x / 100)
+                        } else if x % 10 == 0 {
+                            // one trailing 0, divide by 10 and write
+                            itoa::fmt(f, x / 10)
+                        } else {
+                            itoa::fmt(f, x)
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+mod _naive {
+    use std::fmt;
+
+    pub struct Quality(pub(crate) u16);
+
+    impl fmt::Display for Quality {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            match self.0 {
+                0 => f.write_str("0"),
+                1000 => f.write_str("1"),
+
+                x => {
+                    write!(f, "{}", format!("{:03}", x).trim_end_matches('0'))
+                }
+            }
+        }
+    }
+}

actix-http/src/header/mod.rs

@@ -38,13 +38,14 @@ pub mod map;
 mod shared;
 mod utils;
 
-#[doc(hidden)]
-pub use self::shared::*;
-
 pub use self::as_name::AsHeaderName;
 pub use self::into_pair::IntoHeaderPair;
 pub use self::into_value::IntoHeaderValue;
 pub use self::map::HeaderMap;
+pub use self::shared::{
+    parse_extended_value, q, Charset, ContentEncoding, ExtendedValue, HttpDate,
+    LanguageTag, Quality, QualityItem,
+};
 pub use self::utils::{
     fmt_comma_delimited, from_comma_delimited, from_one_raw_str, http_percent_encode,
 };

actix-http/src/header/shared/extended.rs

@@ -1,4 +1,4 @@
-// Originally from hyper v0.11.27 src/header/parsing.rs
+//! Originally taken from `hyper::header::parsing`.
 
 use std::{fmt, str::FromStr};
 

actix-http/src/header/shared/mod.rs

@@ -4,11 +4,13 @@ mod charset;
 mod content_encoding;
 mod extended;
 mod http_date;
+mod quality;
 mod quality_item;
 
 pub use self::charset::Charset;
 pub use self::content_encoding::ContentEncoding;
 pub use self::extended::{parse_extended_value, ExtendedValue};
 pub use self::http_date::HttpDate;
-pub use self::quality_item::{q, qitem, Quality, QualityItem};
+pub use self::quality::{q, Quality};
+pub use self::quality_item::QualityItem;
 pub use language_tags::LanguageTag;

actix-http/src/header/shared/quality.rs

@@ -0,0 +1,208 @@
+use std::{
+    convert::{TryFrom, TryInto},
+    fmt,
+};
+
+use derive_more::{Display, Error};
+
+const MAX_QUALITY_INT: u16 = 1000;
+const MAX_QUALITY_FLOAT: f32 = 1.0;
+
+/// Represents a quality used in q-factor values.
+///
+/// The default value is equivalent to `q=1.0` (the [max](Self::MAX) value).
+///
+/// # Implementation notes
+/// The quality value is defined as a number between 0.0 and 1.0 with three decimal places.
+/// This means there are 1001 possible values. Since floating point numbers are not exact and the
+/// smallest floating point data type (`f32`) consumes four bytes, we use an `u16` value to store
+/// the quality internally.
+///
+/// [RFC 7231 §5.3.1] gives more information on quality values in HTTP header fields.
+///
+/// # Examples
+/// ```
+/// use actix_http::header::{Quality, q};
+/// assert_eq!(q(1.0), Quality::MAX);
+///
+/// assert_eq!(q(0.42).to_string(), "0.42");
+/// assert_eq!(q(1.0).to_string(), "1");
+/// assert_eq!(Quality::MIN.to_string(), "0");
+/// ```
+///
+/// [RFC 7231 §5.3.1]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.1
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub struct Quality(pub(super) u16);
+
+impl Quality {
+    /// The maximum quality value, equivalent to `q=1.0`.
+    pub const MAX: Quality = Quality(MAX_QUALITY_INT);
+
+    /// The minimum quality value, equivalent to `q=0.0`.
+    pub const MIN: Quality = Quality(0);
+
+    /// Converts a float in the range 0.0–1.0 to a `Quality`.
+    ///
+    /// Intentionally private. External uses should rely on the `TryFrom` impl.
+    ///
+    /// # Panics
+    /// Panics in debug mode when value is not in the range 0.0 <= n <= 1.0.
+    fn from_f32(value: f32) -> Self {
+        // Check that `value` is within range should be done before calling this method.
+        // Just in case, this debug_assert should catch if we were forgetful.
+        debug_assert!(
+            (0.0f32..=1.0f32).contains(&value),
+            "q value must be between 0.0 and 1.0"
+        );
+
+        Quality((value * MAX_QUALITY_INT as f32) as u16)
+    }
+}
+
+/// The default value is [`Quality::MAX`].
+impl Default for Quality {
+    fn default() -> Quality {
+        Quality::MAX
+    }
+}
+
+impl fmt::Display for Quality {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.0 {
+            0 => f.write_str("0"),
+            MAX_QUALITY_INT => f.write_str("1"),
+
+            // some number in the range 1–999
+            x => {
+                f.write_str("0.")?;
+
+                // This implementation avoids string allocation for removing trailing zeroes.
+                // In benchmarks it is twice as fast as approach using something like
+                // `format!("{}").trim_end_matches('0')` for non-fast-path quality values.
+
+                if x < 10 {
+                    // x in is range 1–9
+
+                    f.write_str("00")?;
+
+                    // 0 is already handled so it's not possible to have a trailing 0 in this range
+                    // we can just write the integer
+                    itoa::fmt(f, x)
+                } else if x < 100 {
+                    // x in is range 10–99
+
+                    f.write_str("0")?;
+
+                    if x % 10 == 0 {
+                        // trailing 0, divide by 10 and write
+                        itoa::fmt(f, x / 10)
+                    } else {
+                        itoa::fmt(f, x)
+                    }
+                } else {
+                    // x is in range 100–999
+
+                    if x % 100 == 0 {
+                        // two trailing 0s, divide by 100 and write
+                        itoa::fmt(f, x / 100)
+                    } else if x % 10 == 0 {
+                        // one trailing 0, divide by 10 and write
+                        itoa::fmt(f, x / 10)
+                    } else {
+                        itoa::fmt(f, x)
+                    }
+                }
+            }
+        }
+    }
+}
+
+#[derive(Debug, Clone, Display, Error)]
+#[display(fmt = "quality out of bounds")]
+#[non_exhaustive]
+pub struct QualityOutOfBounds;
+
+impl TryFrom<f32> for Quality {
+    type Error = QualityOutOfBounds;
+
+    #[inline]
+    fn try_from(value: f32) -> Result<Self, Self::Error> {
+        if (0.0..=MAX_QUALITY_FLOAT).contains(&value) {
+            Ok(Quality::from_f32(value))
+        } else {
+            Err(QualityOutOfBounds)
+        }
+    }
+}
+
+/// Convenience function to create a [`Quality`] from an `f32` (0.0–1.0).
+///
+/// Not recommended for use with user input. Rely on the `TryFrom` impls where possible.
+///
+/// # Panics
+/// Panics if value is out of range.
+///
+/// # Examples
+/// ```
+/// # use actix_http::header::{q, Quality};
+/// let q1 = q(1.0);
+/// assert_eq!(q1, Quality::MAX);
+///
+/// let q2 = q(0.0);
+/// assert_eq!(q2, Quality::MIN);
+///
+/// let q3 = q(0.42);
+/// ```
+///
+/// An out-of-range `f32` quality will panic.
+/// ```should_panic
+/// # use actix_http::header::q;
+/// let _q2 = q(1.42);
+/// ```
+#[inline]
+pub fn q<T>(quality: T) -> Quality
+where
+    T: TryInto<Quality>,
+    T::Error: fmt::Debug,
+{
+    quality.try_into().expect("quality value was out of bounds")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn q_helper() {
+        assert_eq!(q(0.5), Quality(500));
+    }
+
+    #[test]
+    fn display_output() {
+        assert_eq!(q(0.0).to_string(), "0");
+        assert_eq!(q(1.0).to_string(), "1");
+        assert_eq!(q(0.001).to_string(), "0.001");
+        assert_eq!(q(0.5).to_string(), "0.5");
+        assert_eq!(q(0.22).to_string(), "0.22");
+        assert_eq!(q(0.123).to_string(), "0.123");
+        assert_eq!(q(0.999).to_string(), "0.999");
+
+        for x in 0..=1000 {
+            // if trailing zeroes are handled correctly, we would not expect the serialized length
+            // to ever exceed "0." + 3 decimal places = 5 in length
+            assert!(q(x as f32 / 1000.0).to_string().len() <= 5);
+        }
+    }
+
+    #[test]
+    #[should_panic]
+    fn negative_quality() {
+        q(-1.0);
+    }
+
+    #[test]
+    #[should_panic]
+    fn quality_out_of_bounds() {
+        q(2.0);
+    }
+}