From 8f6ce453ded9b5d9112a4e6e210d91441cb1846e Mon Sep 17 00:00:00 2001 From: Pyfisch Date: Sun, 22 Feb 2015 16:51:53 +0100 Subject: [PATCH] refactor(headers): Use u16 based newtype for quality value Using floating point numbers is problematic because comparison is inexact. They also take more space than integral numbers in this case. Add `FromPrimitve`, `ToPrimitive` and `Default` traits to quality newtype. Closes: #330 BREAKING_CHANGE: Replace f32 quality values in quality items with a Quality(u16) newtype. Valid values are from 0 to 1000. --- src/header/common/accept.rs | 5 +- src/header/common/accept_encoding.rs | 6 +- src/header/common/accept_language.rs | 76 ++++----- src/header/mod.rs | 6 +- src/header/shared/mod.rs | 2 +- src/header/shared/quality_item.rs | 221 ++++++++++++++++++--------- 6 files changed, 202 insertions(+), 114 deletions(-) diff --git a/src/header/common/accept.rs b/src/header/common/accept.rs index d07d40ad..6b44f39f 100644 --- a/src/header/common/accept.rs +++ b/src/header/common/accept.rs @@ -33,7 +33,8 @@ impl_list_header!(Accept, mod tests { use mime::*; - use header::{Header, QualityItem, qitem}; + use header::{Header, Quality, QualityItem, qitem}; + use super::Accept; #[test] @@ -49,7 +50,7 @@ mod tests { fn test_parse_header_with_quality() { let a: Accept = Header::parse_header([b"text/plain; charset=utf-8; q=0.5".to_vec()].as_slice()).unwrap(); let b = Accept(vec![ - QualityItem::new(Mime(TopLevel::Text, SubLevel::Plain, vec![(Attr::Charset, Value::Utf8)]), 0.5f32), + QualityItem::new(Mime(TopLevel::Text, SubLevel::Plain, vec![(Attr::Charset, Value::Utf8)]), Quality(500)), ]); assert_eq!(a, b); } diff --git a/src/header/common/accept_encoding.rs b/src/header/common/accept_encoding.rs index 4e330058..9ee95250 100644 --- a/src/header/common/accept_encoding.rs +++ b/src/header/common/accept_encoding.rs @@ -13,7 +13,7 @@ impl_list_header!(AcceptEncoding, #[cfg(test)] mod tests { - use header::{Encoding, Header, QualityItem}; + use header::{Encoding, Header, qitem, Quality, QualityItem}; use super::*; @@ -21,8 +21,8 @@ mod tests { fn test_parse_header() { let a: AcceptEncoding = Header::parse_header([b"gzip;q=1.0, identity; q=0.5".to_vec()].as_slice()).unwrap(); let b = AcceptEncoding(vec![ - QualityItem{item: Encoding::Gzip, quality: 1f32}, - QualityItem{item: Encoding::Identity, quality: 0.5f32}, + qitem(Encoding::Gzip), + QualityItem::new(Encoding::Identity, Quality(500)), ]); assert_eq!(a, b); } diff --git a/src/header/common/accept_language.rs b/src/header/common/accept_language.rs index 1a3dc206..789ea03a 100644 --- a/src/header/common/accept_language.rs +++ b/src/header/common/accept_language.rs @@ -46,41 +46,45 @@ impl_list_header!(AcceptLanguage, "Accept-Language", Vec>); +#[cfg(test)] +mod tests { + use header::{Header, qitem, Quality, QualityItem}; + + use super::*; + + #[test] + fn test_parse_header() { + let a: AcceptLanguage = Header::parse_header( + [b"en-us;q=1.0, en;q=0.5, fr".to_vec()].as_slice()).unwrap(); + let b = AcceptLanguage(vec![ + qitem(Language{primary: "en".to_string(), sub: Some("us".to_string())}), + QualityItem::new(Language{primary: "en".to_string(), sub: None}, + Quality(500)), + qitem(Language{primary: "fr".to_string(), sub: None}), + ]); + assert_eq!(format!("{}", a), format!("{}", b)); + assert_eq!(a, b); + } + + #[test] + fn test_display() { + assert_eq!("en".to_string(), + format!("{}", Language{primary: "en".to_string(), + sub: None})); + assert_eq!("en-us".to_string(), + format!("{}", Language{primary: "en".to_string(), + sub: Some("us".to_string())})); + } + + #[test] + fn test_from_str() { + assert_eq!(Language { primary: "en".to_string(), sub: None }, + "en".parse().unwrap()); + assert_eq!(Language { primary: "en".to_string(), + sub: Some("us".to_string()) }, + "en-us".parse().unwrap()); + } +} + bench_header!(bench, AcceptLanguage, { vec![b"en-us;q=1.0, en;q=0.5, fr".to_vec()] }); - -#[test] -fn test_parse_header() { - let a: AcceptLanguage = header::Header::parse_header( - [b"en-us;q=1.0, en;q=0.5, fr".to_vec()].as_slice()).unwrap(); - let b = AcceptLanguage(vec![ - QualityItem { item: Language{primary: "en".to_string(), - sub: Some("us".to_string())}, - quality: 1f32 }, - QualityItem { item: Language{primary: "en".to_string(), sub: None}, - quality: 0.5f32 }, - QualityItem { item: Language{primary: "fr".to_string(), sub: None}, - quality: 1f32 }, - ]); - assert_eq!(format!("{}", a), format!("{}", b)); - assert_eq!(a, b); -} - -#[test] -fn test_display() { - assert_eq!("en".to_string(), - format!("{}", Language{primary: "en".to_string(), - sub: None})); - assert_eq!("en-us".to_string(), - format!("{}", Language{primary: "en".to_string(), - sub: Some("us".to_string())})); -} - -#[test] -fn test_from_str() { - assert_eq!(Language { primary: "en".to_string(), sub: None }, - "en".parse().unwrap()); - assert_eq!(Language { primary: "en".to_string(), - sub: Some("us".to_string()) }, - "en-us".parse().unwrap()); -} diff --git a/src/header/mod.rs b/src/header/mod.rs index 46c9dd82..2a6eacb7 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -21,7 +21,7 @@ use unicase::UniCase; use self::cell::OptCell; use {http, HttpResult, HttpError}; -pub use self::shared::{Encoding, EntityTag, QualityItem, qitem}; +pub use self::shared::{Encoding, EntityTag, Quality, QualityItem, qitem}; pub use self::common::*; mod cell; @@ -540,7 +540,7 @@ mod tests { use mime::TopLevel::Text; use mime::SubLevel::Plain; use super::{Headers, Header, HeaderFormat, ContentLength, ContentType, - Accept, Host, QualityItem}; + Accept, Host, qitem}; use test::Bencher; @@ -562,7 +562,7 @@ mod tests { #[test] fn test_accept() { - let text_plain = QualityItem{item: Mime(Text, Plain, vec![]), quality: 1f32}; + let text_plain = qitem(Mime(Text, Plain, vec![])); let application_vendor = "application/vnd.github.v3.full+json; q=0.5".parse().unwrap(); let accept = Header::parse_header([b"text/plain".to_vec()].as_slice()); diff --git a/src/header/shared/mod.rs b/src/header/shared/mod.rs index 7c12bd48..dd81208a 100644 --- a/src/header/shared/mod.rs +++ b/src/header/shared/mod.rs @@ -1,6 +1,6 @@ pub use self::encoding::Encoding; pub use self::entity::EntityTag; -pub use self::quality_item::{QualityItem, qitem}; +pub use self::quality_item::{Quality, QualityItem, qitem}; mod encoding; mod entity; diff --git a/src/header/shared/quality_item.rs b/src/header/shared/quality_item.rs index 199ef9a7..814668d4 100644 --- a/src/header/shared/quality_item.rs +++ b/src/header/shared/quality_item.rs @@ -3,10 +3,79 @@ //! [RFC7231 Section 5.3.1](https://tools.ietf.org/html/rfc7231#section-5.3.1) //! gives more information on quality values in HTTP header fields. -use std::fmt; -use std::str; use std::cmp; -#[cfg(test)] use super::encoding::*; +use std::default::Default; +use std::fmt; +use std::num::{FromPrimitive, ToPrimitive}; +use std::str; + +/// Represents a quality used in quality values. +/// +/// `Quality` should only be created using the `FromPrimitve` trait methods `from_f32` and +/// `from_f64`, they take a value between 0.0 and 1.0. To create a quality with the value 1.0, the +/// default you can use `let q: Quality = Default::default()`. +/// +/// # Implementation notes +/// The quality value is defined as a number between 0 and 1 with three decimal places. This means +/// there are 1000 possible values. Since floating point numbers are not exact and the smallest +/// floating point data type (`f32`) consumes four bytes, hyper uses an `u16` value to store the +/// quality internally. For performance reasons you may set quality directly to a value between +/// 0 and 1000 e.g. `Quality(532)` matches the quality `q=0.532`. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub struct Quality(pub u16); + +impl fmt::Display for Quality { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if self.0 == 1000 { + write!(f, "") + } else { + write!(f, "; q=0.{}", format!("{:03}", self.0).trim_right_matches('0')) + } + } +} + +impl FromPrimitive for Quality { + fn from_i64(n: i64) -> Option { + match n >= 0 { + true => FromPrimitive::from_u64(n as u64), + false => None, + } + } + + fn from_u64(n: u64) -> Option { + match n <= 1000 { + true => Some(Quality(n as u16)), + false => None, + } + } + + fn from_f64(n: f64) -> Option { + match n >= 0f64 && n <= 1f64 { + true => Some(Quality((n * 1000f64) as u16)), + false => None, + } + } +} + +impl ToPrimitive for Quality { + fn to_i64(&self) -> Option { + Some(self.0 as i64) + } + + fn to_u64(&self) -> Option { + Some(self.0 as u64) + } + + fn to_f64(&self) -> Option { + Some((self.0 as f64) / 1000f64) + } +} + +impl Default for Quality { + fn default() -> Quality { + Quality(1000) + } +} /// Represents an item with a quality value as defined in /// [RFC7231](https://tools.ietf.org/html/rfc7231#section-5.3.1). @@ -15,14 +84,14 @@ pub struct QualityItem { /// The actual contents of the field. pub item: T, /// The quality (client or server preference) for the value. - pub quality: f32, + pub quality: Quality, } impl QualityItem { /// Creates a new `QualityItem` from an item and a quality. /// The item can be of any type. /// The quality should be a value in the range [0, 1]. - pub fn new(item: T, quality: f32) -> QualityItem { + pub fn new(item: T, quality: Quality) -> QualityItem { QualityItem{item: item, quality: quality} } } @@ -35,12 +104,7 @@ impl cmp::PartialOrd for QualityItem { impl fmt::Display for QualityItem { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self.quality == 1.0 { - write!(f, "{}", self.item) - } else { - write!(f, "{}; q={}", self.item, - format!("{:.3}", self.quality).trim_right_matches(&['0', '.'][..])) - } + write!(f, "{}{}", self.item, format!("{}", self.quality)) } } @@ -59,8 +123,7 @@ impl str::FromStr for QualityItem { if q_part.len() > 5 { return Err(()); } - let x: Result = q_part.parse(); - match x { + match q_part.parse::() { Ok(q_value) => { if 0f32 <= q_value && q_value <= 1f32 { quality = q_value; @@ -73,11 +136,8 @@ impl str::FromStr for QualityItem { } } } - let x: Result = raw_item.parse(); - match x { - Ok(item) => { - Ok(QualityItem{ item: item, quality: quality, }) - }, + match raw_item.parse::() { + Ok(item) => Ok(QualityItem::new(item, FromPrimitive::from_f32(quality).unwrap())), Err(_) => return Err(()), } } @@ -86,58 +146,81 @@ impl str::FromStr for QualityItem { /// Convinience function to wrap a value in a `QualityItem` /// Sets `q` to the default 1.0 pub fn qitem(item: T) -> QualityItem { - QualityItem::new(item, 1.0) + QualityItem::new(item, Default::default()) } -#[test] -fn test_quality_item_show1() { - let x = qitem(Chunked); - assert_eq!(format!("{}", x), "chunked"); -} -#[test] -fn test_quality_item_show2() { - let x = QualityItem::new(Chunked, 0.001); - assert_eq!(format!("{}", x), "chunked; q=0.001"); -} -#[test] -fn test_quality_item_show3() { - // Custom value - let x = QualityItem{ - item: EncodingExt("identity".to_string()), - quality: 0.5f32, - }; - assert_eq!(format!("{}", x), "identity; q=0.5"); -} +#[cfg(test)] +mod tests { + use std::num::FromPrimitive; -#[test] -fn test_quality_item_from_str1() { - let x: Result, ()> = "chunked".parse(); - assert_eq!(x.unwrap(), QualityItem{ item: Chunked, quality: 1f32, }); -} -#[test] -fn test_quality_item_from_str2() { - let x: Result, ()> = "chunked; q=1".parse(); - assert_eq!(x.unwrap(), QualityItem{ item: Chunked, quality: 1f32, }); -} -#[test] -fn test_quality_item_from_str3() { - let x: Result, ()> = "gzip; q=0.5".parse(); - assert_eq!(x.unwrap(), QualityItem{ item: Gzip, quality: 0.5f32, }); -} -#[test] -fn test_quality_item_from_str4() { - let x: Result, ()> = "gzip; q=0.273".parse(); - assert_eq!(x.unwrap(), QualityItem{ item: Gzip, quality: 0.273f32, }); -} -#[test] -fn test_quality_item_from_str5() { - let x: Result, ()> = "gzip; q=0.2739999".parse(); - assert_eq!(x, Err(())); -} -#[test] -fn test_quality_item_ordering() { - let x: QualityItem = "gzip; q=0.5".parse().ok().unwrap(); - let y: QualityItem = "gzip; q=0.273".parse().ok().unwrap(); - let comparision_result: bool = x.gt(&y); - assert_eq!(comparision_result, true) + use super::*; + use super::super::encoding::*; + + #[test] + fn test_quality_item_show1() { + let x = qitem(Chunked); + assert_eq!(format!("{}", x), "chunked"); + } + #[test] + fn test_quality_item_show2() { + let x = QualityItem::new(Chunked, Quality(1)); + assert_eq!(format!("{}", x), "chunked; q=0.001"); + } + #[test] + fn test_quality_item_show3() { + // Custom value + let x = QualityItem{ + item: EncodingExt("identity".to_string()), + quality: Quality(500), + }; + assert_eq!(format!("{}", x), "identity; q=0.5"); + } + + #[test] + fn test_quality_item_from_str1() { + let x: Result, ()> = "chunked".parse(); + assert_eq!(x.unwrap(), QualityItem{ item: Chunked, quality: Quality(1000), }); + } + #[test] + fn test_quality_item_from_str2() { + let x: Result, ()> = "chunked; q=1".parse(); + assert_eq!(x.unwrap(), QualityItem{ item: Chunked, quality: Quality(1000), }); + } + #[test] + fn test_quality_item_from_str3() { + let x: Result, ()> = "gzip; q=0.5".parse(); + assert_eq!(x.unwrap(), QualityItem{ item: Gzip, quality: Quality(500), }); + } + #[test] + fn test_quality_item_from_str4() { + let x: Result, ()> = "gzip; q=0.273".parse(); + assert_eq!(x.unwrap(), QualityItem{ item: Gzip, quality: Quality(273), }); + } + #[test] + fn test_quality_item_from_str5() { + let x: Result, ()> = "gzip; q=0.2739999".parse(); + assert_eq!(x, Err(())); + } + #[test] + fn test_quality_item_ordering() { + let x: QualityItem = "gzip; q=0.5".parse().ok().unwrap(); + let y: QualityItem = "gzip; q=0.273".parse().ok().unwrap(); + let comparision_result: bool = x.gt(&y); + assert_eq!(comparision_result, true) + } + #[test] + fn test_quality() { + assert_eq!(Some(Quality(421)), FromPrimitive::from_f64(0.421f64)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_f32(0.421f32)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_i16(421i16)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_i32(421i32)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_i64(421i64)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_u16(421u16)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_u32(421u32)); + assert_eq!(Some(Quality(421)), FromPrimitive::from_u64(421u64)); + + assert_eq!(None::, FromPrimitive::from_i16(-5i16)); + assert_eq!(None::, FromPrimitive::from_i32(5000i32)); + assert_eq!(None::, FromPrimitive::from_f32(2.5f32)); + } }