Skip to content

Commit

Permalink
refactor(header): make Quality an opaque struct
Browse files Browse the repository at this point in the history
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)`.
  • Loading branch information
seanmonstar committed Apr 26, 2017
1 parent 316c6fa commit a464495
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 44 deletions.
14 changes: 7 additions & 7 deletions src/header/common/accept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
///
/// let mut headers = Headers::new();
Expand All @@ -64,11 +64,11 @@ header! {
/// qitem(Mime(TopLevel::Application,
/// SubLevel::Ext("xhtml+xml".to_owned()), vec![])),
/// QualityItem::new(Mime(TopLevel::Application, SubLevel::Xml, vec![]),
/// Quality(900)),
/// q(900)),
/// qitem(Mime(TopLevel::Image,
/// SubLevel::Ext("webp".to_owned()), vec![])),
/// QualityItem::new(Mime(TopLevel::Star, SubLevel::Star, vec![]),
/// Quality(800))
/// q(800))
/// ])
/// );
/// ```
Expand All @@ -85,18 +85,18 @@ header! {
// test1,
// vec![b"audio/*; q=0.2, audio/basic"],
// 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![])),
// ])));
test_header!(
test2,
vec![b"text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c"],
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![])),
QualityItem::new(
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![])),
])));
// Custom tests
Expand All @@ -112,7 +112,7 @@ header! {
Some(Accept(vec![
QualityItem::new(Mime(TopLevel::Text,
SubLevel::Plain, vec![(Attr::Charset, Value::Utf8)]),
Quality(500)),
q(500)),
])));

#[test]
Expand Down
6 changes: 3 additions & 3 deletions src/header/common/accept_charset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
/// headers.set(
/// AcceptCharset(vec![
/// QualityItem::new(Charset::Us_Ascii, Quality(900)),
/// QualityItem::new(Charset::Iso_8859_10, Quality(200)),
/// QualityItem::new(Charset::Us_Ascii, q(900)),
/// QualityItem::new(Charset::Iso_8859_10, q(200)),
/// ])
/// );
/// ```
Expand Down
6 changes: 3 additions & 3 deletions src/header/common/accept_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
/// headers.set(
/// AcceptEncoding(vec![
/// qitem(Encoding::Chunked),
/// QualityItem::new(Encoding::Gzip, Quality(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), Quality(0)),
/// QualityItem::new(Encoding::Gzip, q(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), q(0)),
/// ])
/// );
/// ```
Expand Down
8 changes: 4 additions & 4 deletions src/header/common/accept_language.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ header! {
/// ```
/// # extern crate hyper;
/// # #[macro_use] extern crate language_tags;
/// # use hyper::header::{Headers, AcceptLanguage, QualityItem, Quality, qitem};
/// # use hyper::header::{Headers, AcceptLanguage, QualityItem, q, qitem};
/// #
/// # fn main() {
/// let mut headers = Headers::new();
/// headers.set(
/// AcceptLanguage(vec![
/// qitem(langtag!(da)),
/// QualityItem::new(langtag!(en;;;GB), Quality(800)),
/// QualityItem::new(langtag!(en), Quality(700)),
/// QualityItem::new(langtag!(en;;;GB), q(800)),
/// QualityItem::new(langtag!(en), q(700)),
/// ])
/// );
/// # }
Expand All @@ -59,7 +59,7 @@ header! {
test2, vec![b"en-US, en; q=0.5, fr"],
Some(AcceptLanguage(vec![
qitem("en-US".parse().unwrap()),
QualityItem::new("en".parse().unwrap(), Quality(500)),
QualityItem::new("en".parse().unwrap(), q(500)),
qitem("fr".parse().unwrap()),
])));
}
Expand Down
6 changes: 3 additions & 3 deletions src/header/common/te.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
/// headers.set(
/// TE(vec![
/// qitem(Encoding::Trailers),
/// QualityItem::new(Encoding::Gzip, Quality(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), Quality(0)),
/// QualityItem::new(Encoding::Gzip, q(600)),
/// QualityItem::new(Encoding::EncodingExt("*".to_owned()), q(0)),
/// ])
/// );
/// ```
Expand Down
86 changes: 62 additions & 24 deletions src/header/shared/quality_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use std::default::Default;
use std::fmt;
use std::str;

use self::internal::IntoQuality;

/// Represents a quality used in quality values.
///
/// Can be created with the `q` function.
Expand All @@ -19,17 +21,7 @@ use std::str;
/// [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.
#[derive(Copy, 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 {
match self.0 {
1000 => Ok(()),
0 => f.write_str("; q=0"),
x => write!(f, "; q=0.{}", format!("{:03}", x).trim_right_matches('0'))
}
}
}
pub struct Quality(u16);

impl Default for Quality {
fn default() -> Quality {
Expand Down Expand Up @@ -67,7 +59,12 @@ impl<T: PartialEq> cmp::PartialOrd for QualityItem<T> {

impl<T: fmt::Display> fmt::Display for QualityItem<T> {
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'))
}
}
}

Expand Down Expand Up @@ -113,6 +110,7 @@ impl<T: str::FromStr> str::FromStr for QualityItem<T> {
}
}

#[inline]
fn from_f32(f: f32) -> Quality {
// this function is only used internally. A check that `f` is within range
// should be done before calling this method. Just in case, this
Expand All @@ -127,10 +125,45 @@ pub fn qitem<T>(item: T) -> QualityItem<T> {
QualityItem::new(item, Default::default())
}

/// Convenience function to create a `Quality` from a float.
pub fn q(f: f32) -> Quality {
assert!(f >= 0f32 && f <= 1f32, "q value must be between 0.0 and 1.0");
from_f32(f)
/// Convenience function to create a `Quality` from a float or integer.
///
/// Implemented for `u16` and `f32`. Panics if value is out of range.
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)]
Expand All @@ -139,17 +172,17 @@ mod tests {
use super::super::encoding::*;

#[test]
fn test_quality_item_show1() {
fn test_quality_item_fmt_q_1() {
let x = qitem(Chunked);
assert_eq!(format!("{}", x), "chunked");
}
#[test]
fn test_quality_item_show2() {
fn test_quality_item_fmt_q_0001() {
let x = QualityItem::new(Chunked, Quality(1));
assert_eq!(format!("{}", x), "chunked; q=0.001");
}
#[test]
fn test_quality_item_show3() {
fn test_quality_item_fmt_q_05() {
// Custom value
let x = QualityItem{
item: EncodingExt("identity".to_owned()),
Expand All @@ -158,6 +191,16 @@ mod tests {
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]
fn test_quality_item_from_str1() {
let x: ::Result<QualityItem<Encoding>> = "chunked".parse();
Expand Down Expand Up @@ -201,11 +244,6 @@ mod tests {
assert_eq!(q(0.5), Quality(500));
}

#[test]
fn test_quality2() {
assert_eq!(format!("{}", q(0.0)), "; q=0");
}

#[test]
#[should_panic] // FIXME - 32-bit msvc unwinding broken
#[cfg_attr(all(target_arch="x86", target_env="msvc"), ignore)]
Expand Down

0 comments on commit a464495

Please sign in to comment.