refactor(header): make Quality an opaque struct

This makes the `u16` in `Quality` private, since it only has a valid
range of 0-1000, and can't be enforced in public. The `q` function now
allows both `f32`s and `u16`s to construct a `Quality`.

BREAKING CHANGE: Any use of `Quality(num)` should change to `q(num)`.
This commit is contained in:
Sean McArthur
2017-04-25 17:02:25 -07:00
parent 316c6fad30
commit a4644959b0
6 changed files with 82 additions and 44 deletions

View File

@@ -53,7 +53,7 @@ header! {
/// ); /// );
/// ``` /// ```
/// ``` /// ```
/// use hyper::header::{Headers, Accept, QualityItem, Quality, qitem}; /// use hyper::header::{Headers, Accept, QualityItem, q, qitem};
/// use hyper::mime::{Mime, TopLevel, SubLevel}; /// use hyper::mime::{Mime, TopLevel, SubLevel};
/// ///
/// let mut headers = Headers::new(); /// let mut headers = Headers::new();
@@ -64,11 +64,11 @@ header! {
/// qitem(Mime(TopLevel::Application, /// qitem(Mime(TopLevel::Application,
/// SubLevel::Ext("xhtml+xml".to_owned()), vec![])), /// SubLevel::Ext("xhtml+xml".to_owned()), vec![])),
/// QualityItem::new(Mime(TopLevel::Application, SubLevel::Xml, vec![]), /// QualityItem::new(Mime(TopLevel::Application, SubLevel::Xml, vec![]),
/// Quality(900)), /// q(900)),
/// qitem(Mime(TopLevel::Image, /// qitem(Mime(TopLevel::Image,
/// SubLevel::Ext("webp".to_owned()), vec![])), /// SubLevel::Ext("webp".to_owned()), vec![])),
/// QualityItem::new(Mime(TopLevel::Star, SubLevel::Star, vec![]), /// QualityItem::new(Mime(TopLevel::Star, SubLevel::Star, vec![]),
/// Quality(800)) /// q(800))
/// ]) /// ])
/// ); /// );
/// ``` /// ```
@@ -85,18 +85,18 @@ header! {
// test1, // test1,
// vec![b"audio/*; q=0.2, audio/basic"], // vec![b"audio/*; q=0.2, audio/basic"],
// Some(HeaderField(vec![ // Some(HeaderField(vec![
// QualityItem::new(Mime(TopLevel::Audio, SubLevel::Star, vec![]), Quality(200)), // QualityItem::new(Mime(TopLevel::Audio, SubLevel::Star, vec![]), q(200)),
// qitem(Mime(TopLevel::Audio, SubLevel::Ext("basic".to_owned()), vec![])), // qitem(Mime(TopLevel::Audio, SubLevel::Ext("basic".to_owned()), vec![])),
// ]))); // ])));
test_header!( test_header!(
test2, test2,
vec![b"text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c"], vec![b"text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c"],
Some(HeaderField(vec![ Some(HeaderField(vec![
QualityItem::new(Mime(TopLevel::Text, SubLevel::Plain, vec![]), Quality(500)), QualityItem::new(Mime(TopLevel::Text, SubLevel::Plain, vec![]), q(500)),
qitem(Mime(TopLevel::Text, SubLevel::Html, vec![])), qitem(Mime(TopLevel::Text, SubLevel::Html, vec![])),
QualityItem::new( QualityItem::new(
Mime(TopLevel::Text, SubLevel::Ext("x-dvi".to_owned()), vec![]), Mime(TopLevel::Text, SubLevel::Ext("x-dvi".to_owned()), vec![]),
Quality(800)), q(800)),
qitem(Mime(TopLevel::Text, SubLevel::Ext("x-c".to_owned()), vec![])), qitem(Mime(TopLevel::Text, SubLevel::Ext("x-c".to_owned()), vec![])),
]))); ])));
// Custom tests // Custom tests
@@ -112,7 +112,7 @@ header! {
Some(Accept(vec![ Some(Accept(vec![
QualityItem::new(Mime(TopLevel::Text, QualityItem::new(Mime(TopLevel::Text,
SubLevel::Plain, vec![(Attr::Charset, Value::Utf8)]), SubLevel::Plain, vec![(Attr::Charset, Value::Utf8)]),
Quality(500)), q(500)),
]))); ])));
#[test] #[test]

View File

@@ -29,13 +29,13 @@ header! {
/// ); /// );
/// ``` /// ```
/// ``` /// ```
/// use hyper::header::{Headers, AcceptCharset, Charset, Quality, QualityItem}; /// use hyper::header::{Headers, AcceptCharset, Charset, q, QualityItem};
/// ///
/// let mut headers = Headers::new(); /// let mut headers = Headers::new();
/// headers.set( /// headers.set(
/// AcceptCharset(vec![ /// AcceptCharset(vec![
/// QualityItem::new(Charset::Us_Ascii, Quality(900)), /// QualityItem::new(Charset::Us_Ascii, q(900)),
/// QualityItem::new(Charset::Iso_8859_10, Quality(200)), /// QualityItem::new(Charset::Iso_8859_10, q(200)),
/// ]) /// ])
/// ); /// );
/// ``` /// ```

View File

@@ -45,14 +45,14 @@ header! {
/// ); /// );
/// ``` /// ```
/// ``` /// ```
/// use hyper::header::{Headers, AcceptEncoding, Encoding, QualityItem, Quality, qitem}; /// use hyper::header::{Headers, AcceptEncoding, Encoding, QualityItem, q, qitem};
/// ///
/// let mut headers = Headers::new(); /// let mut headers = Headers::new();
/// headers.set( /// headers.set(
/// AcceptEncoding(vec![ /// AcceptEncoding(vec![
/// qitem(Encoding::Chunked), /// qitem(Encoding::Chunked),
/// QualityItem::new(Encoding::Gzip, Quality(600)), /// QualityItem::new(Encoding::Gzip, q(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), Quality(0)), /// QualityItem::new(Encoding::EncodingExt("*".to_owned()), q(0)),
/// ]) /// ])
/// ); /// );
/// ``` /// ```

View File

@@ -36,15 +36,15 @@ header! {
/// ``` /// ```
/// # extern crate hyper; /// # extern crate hyper;
/// # #[macro_use] extern crate language_tags; /// # #[macro_use] extern crate language_tags;
/// # use hyper::header::{Headers, AcceptLanguage, QualityItem, Quality, qitem}; /// # use hyper::header::{Headers, AcceptLanguage, QualityItem, q, qitem};
/// # /// #
/// # fn main() { /// # fn main() {
/// let mut headers = Headers::new(); /// let mut headers = Headers::new();
/// headers.set( /// headers.set(
/// AcceptLanguage(vec![ /// AcceptLanguage(vec![
/// qitem(langtag!(da)), /// qitem(langtag!(da)),
/// QualityItem::new(langtag!(en;;;GB), Quality(800)), /// QualityItem::new(langtag!(en;;;GB), q(800)),
/// QualityItem::new(langtag!(en), Quality(700)), /// QualityItem::new(langtag!(en), q(700)),
/// ]) /// ])
/// ); /// );
/// # } /// # }
@@ -59,7 +59,7 @@ header! {
test2, vec![b"en-US, en; q=0.5, fr"], test2, vec![b"en-US, en; q=0.5, fr"],
Some(AcceptLanguage(vec![ Some(AcceptLanguage(vec![
qitem("en-US".parse().unwrap()), qitem("en-US".parse().unwrap()),
QualityItem::new("en".parse().unwrap(), Quality(500)), QualityItem::new("en".parse().unwrap(), q(500)),
qitem("fr".parse().unwrap()), qitem("fr".parse().unwrap()),
]))); ])));
} }

View File

@@ -45,14 +45,14 @@ header! {
/// ); /// );
/// ``` /// ```
/// ``` /// ```
/// use hyper::header::{Headers, TE, Encoding, QualityItem, Quality, qitem}; /// use hyper::header::{Headers, TE, Encoding, QualityItem, q, qitem};
/// ///
/// let mut headers = Headers::new(); /// let mut headers = Headers::new();
/// headers.set( /// headers.set(
/// TE(vec![ /// TE(vec![
/// qitem(Encoding::Trailers), /// qitem(Encoding::Trailers),
/// QualityItem::new(Encoding::Gzip, Quality(600)), /// QualityItem::new(Encoding::Gzip, q(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), Quality(0)), /// QualityItem::new(Encoding::EncodingExt("*".to_owned()), q(0)),
/// ]) /// ])
/// ); /// );
/// ``` /// ```

View File

@@ -4,6 +4,8 @@ use std::default::Default;
use std::fmt; use std::fmt;
use std::str; use std::str;
use self::internal::IntoQuality;
/// Represents a quality used in quality values. /// Represents a quality used in quality values.
/// ///
/// Can be created with the `q` function. /// Can be created with the `q` function.
@@ -19,17 +21,7 @@ use std::str;
/// [RFC7231 Section 5.3.1](https://tools.ietf.org/html/rfc7231#section-5.3.1) /// [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. /// gives more information on quality values in HTTP header fields.
#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
pub struct Quality(pub u16); pub struct Quality(u16);
impl fmt::Display for Quality {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.0 {
1000 => Ok(()),
0 => f.write_str("; q=0"),
x => write!(f, "; q=0.{}", format!("{:03}", x).trim_right_matches('0'))
}
}
}
impl Default for Quality { impl Default for Quality {
fn default() -> Quality { fn default() -> Quality {
@@ -67,7 +59,12 @@ impl<T: PartialEq> cmp::PartialOrd for QualityItem<T> {
impl<T: fmt::Display> fmt::Display for QualityItem<T> { impl<T: fmt::Display> fmt::Display for QualityItem<T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}{}", self.item, format!("{}", self.quality)) try!(fmt::Display::fmt(&self.item, f));
match self.quality.0 {
1000 => Ok(()),
0 => f.write_str("; q=0"),
x => write!(f, "; q=0.{}", format!("{:03}", x).trim_right_matches('0'))
}
} }
} }
@@ -113,6 +110,7 @@ impl<T: str::FromStr> str::FromStr for QualityItem<T> {
} }
} }
#[inline]
fn from_f32(f: f32) -> Quality { fn from_f32(f: f32) -> Quality {
// this function is only used internally. A check that `f` is within range // this function is only used internally. A check that `f` is within range
// should be done before calling this method. Just in case, this // should be done before calling this method. Just in case, this
@@ -127,10 +125,45 @@ pub fn qitem<T>(item: T) -> QualityItem<T> {
QualityItem::new(item, Default::default()) QualityItem::new(item, Default::default())
} }
/// Convenience function to create a `Quality` from a float. /// Convenience function to create a `Quality` from a float or integer.
pub fn q(f: f32) -> Quality { ///
assert!(f >= 0f32 && f <= 1f32, "q value must be between 0.0 and 1.0"); /// Implemented for `u16` and `f32`. Panics if value is out of range.
from_f32(f) pub fn q<T: IntoQuality>(val: T) -> Quality {
val.into_quality()
}
mod internal {
use super::Quality;
// TryFrom is probably better, but it's not stable. For now, we want to
// keep the functionality of the `q` function, while allowing it to be
// generic over `f32` and `u16`.
//
// `q` would panic before, so keep that behavior. `TryFrom` can be
// introduced later for a non-panicking conversion.
pub trait IntoQuality: Sealed + Sized {
fn into_quality(self) -> Quality;
}
impl IntoQuality for f32 {
fn into_quality(self) -> Quality {
assert!(self >= 0f32 && self <= 1f32, "float must be between 0.0 and 1.0");
super::from_f32(self)
}
}
impl IntoQuality for u16 {
fn into_quality(self) -> Quality {
assert!(self <= 1000, "u16 must be between 0 and 1000");
Quality(self)
}
}
pub trait Sealed {}
impl Sealed for u16 {}
impl Sealed for f32 {}
} }
#[cfg(test)] #[cfg(test)]
@@ -139,17 +172,17 @@ mod tests {
use super::super::encoding::*; use super::super::encoding::*;
#[test] #[test]
fn test_quality_item_show1() { fn test_quality_item_fmt_q_1() {
let x = qitem(Chunked); let x = qitem(Chunked);
assert_eq!(format!("{}", x), "chunked"); assert_eq!(format!("{}", x), "chunked");
} }
#[test] #[test]
fn test_quality_item_show2() { fn test_quality_item_fmt_q_0001() {
let x = QualityItem::new(Chunked, Quality(1)); let x = QualityItem::new(Chunked, Quality(1));
assert_eq!(format!("{}", x), "chunked; q=0.001"); assert_eq!(format!("{}", x), "chunked; q=0.001");
} }
#[test] #[test]
fn test_quality_item_show3() { fn test_quality_item_fmt_q_05() {
// Custom value // Custom value
let x = QualityItem{ let x = QualityItem{
item: EncodingExt("identity".to_owned()), item: EncodingExt("identity".to_owned()),
@@ -158,6 +191,16 @@ mod tests {
assert_eq!(format!("{}", x), "identity; q=0.5"); assert_eq!(format!("{}", x), "identity; q=0.5");
} }
#[test]
fn test_quality_item_fmt_q_0() {
// Custom value
let x = QualityItem{
item: EncodingExt("identity".to_owned()),
quality: Quality(0),
};
assert_eq!(x.to_string(), "identity; q=0");
}
#[test] #[test]
fn test_quality_item_from_str1() { fn test_quality_item_from_str1() {
let x: ::Result<QualityItem<Encoding>> = "chunked".parse(); let x: ::Result<QualityItem<Encoding>> = "chunked".parse();
@@ -201,11 +244,6 @@ mod tests {
assert_eq!(q(0.5), Quality(500)); assert_eq!(q(0.5), Quality(500));
} }
#[test]
fn test_quality2() {
assert_eq!(format!("{}", q(0.0)), "; q=0");
}
#[test] #[test]
#[should_panic] // FIXME - 32-bit msvc unwinding broken #[should_panic] // FIXME - 32-bit msvc unwinding broken
#[cfg_attr(all(target_arch="x86", target_env="msvc"), ignore)] #[cfg_attr(all(target_arch="x86", target_env="msvc"), ignore)]