hyperium / headers

Typed HTTP Headers from hyper
https://hyper.rs
MIT License
164 stars 83 forks source link

QualityItem Design #7

Open seanmonstar opened 6 years ago

seanmonstar commented 6 years ago

Several headers that a client can send include a list of "quality items", thereby ranking which sorts of representations are more preferable. For example, Accept: image/svg+xml, image/*;q=0.8. These are often used as part of "Content Negotiation". The API design in this library for these headers should be reconsidered here.

eggyal commented 4 years ago

Given that there is already some degree of implementation for this in src/disabled (and taking note of https://github.com/hyperium/headers/pull/67#issuecomment-596712858 together with related commit a514a37), what is the thinking for moving this (also #53) forward from here?

eggyal commented 4 years ago

In terms of external API, perhaps expose a trait:

pub trait QualityItem<T> {
    fn quality(&self) -> u16;
    fn item(&self) -> &T;
}

Which might have a generic implementation along the following lines:

use QualityItem;

pub(crate) struct SimpleQualityItem<T> {
    quality: u16,
    item: T,
}

impl <T> QualityItem<T> for SimpleQualityItem<T> {
    fn quality(&self) -> u16 { self.quality }
    fn item(&self) -> &T { &self.item }
}

Which would enable the relevant header types to be defined like this, without exposing the internal implementation:

use SimpleQualityItem;

struct Encoding { /* ... */ }
pub struct AcceptEncoding(Vec<SimpleQualityItem<Encoding>>);

impl AcceptEncoding {
    pub fn iter(&self) -> impl Iterator<Item = &impl QualityItem<Encoding>> {
        self.0.iter()
    }
}

For headers whose items might carry additional data, such as Accept (which can include "extension parameters" after the quality-value), one could expose and implement additional traits:

use std::collections::HashMap;
use mime::Mime;
use {QualityItem, SimpleQualityItem};

struct AcceptItem {
    content_type_and_quality: SimpleQualityItem<Mime>,
    extension_params: HashMap<String, Option<String>>,
}

impl QualityItem<Mime> for AcceptItem {
    fn quality(&self) -> u16 { self.content_type_and_quality.quality() }
    fn item(&self) -> &Mime { self.content_type_and_quality.item() }
}

pub trait ExtensionParams {
    fn get_extension_param(&self, name: &str) -> Option<Option<&String>>;
}

impl ExtensionParams for AcceptItem {
    fn get_extension_param(&self, name: &str) -> Option<Option<&String>> {
        self.extension_params.get(name).map(Option::as_ref)
    }
}

pub struct Accept(Vec<AcceptItem>);

impl Accept {
    pub fn iter(&self) -> impl Iterator<Item = &(impl QualityItem<Mime> + ExtensionParams)> {
        self.0.iter()
    }
}

Do you think this would be broadly okay, @seanmonstar? If so, I'm happy to work it up and submit a PR.

ParkMyCar commented 4 years ago

70 adds a QualityValue type, it could totally be reworked if necessary, and open to comments on it 🙂