Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client breaking: add initial version of client handlers #449

Draft
wants to merge 1 commit into
base: 0.3.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ keywords = ["trillium", "framework", "async"]
categories = ["web-programming", "web-programming::http-client"]

[features]
json = ["serde_json", "serde", "thiserror"]
default = ["cookies", "json"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider making cookies not be enabled by default. Based on previous experience with tide, once crates in the ecosystem start depending on trillium-client, it's really hard to get all of them using default-features = false, even though many won't need this.

I know that trillium is going for a "just works" API, but having to enable a feature flag to get a more featureful client doesn't seem like too much to ask here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really useful bit of feedback! My initial inclination was for each client handler be their own crate to make totally clear that it doesn't depend on any private state, but convinced myself to limit the proliferation of tiny trillium-client-* crates. I think I may have overcorrected in the other direction. I'll change them to opt-in instead of opt-out before release, or possibly even go for the trillium-client-cookies, trillium-client-follow-redirects, trillium-client-cache crates

Copy link
Collaborator

@joshtriplett joshtriplett Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbr Perhaps a middle ground might make sense: a single trillium-client-handlers crate for all the handlers that don't add dependencies, and either a separate crate or feature flag (or both with re-export) for handlers that do add dependencies. That should reduce proliferation.

cookies = ["dep:cookie_store", "dep:async-lock"]
json = ["dep:serde_json", "dep:serde", "dep:thiserror"]

[dependencies]
encoding_rs = "0.8.33"
Expand All @@ -28,6 +30,9 @@ thiserror = { version = "1.0.50", optional = true }
dashmap = "5.5.3"
crossbeam-queue = "0.3.8"
memchr = "2.6.4"
async-lock = {version = "3.2.0", optional = true }
arc-swap = "1.6.0"
cookie_store = { version = "0.20.0", optional = true }

[dependencies.trillium-http]
path = "../http"
Expand All @@ -36,12 +41,19 @@ version = "0.3.6"

[dev-dependencies]
async-channel = "2.1.0"
async-fs = "2.1.0"
async-global-executor = "2.3.1"
blocking = "1.5.1"
clap = { version = "4.4.10", features = ["derive", "env"] }
clap-verbosity-flag = "2.1.0"
crossbeam = "0.8.2"
env_logger = "0.10.1"
indoc = "2.0.4"
pretty_assertions = "1.4.0"
test-harness = "0.1.1"
trillium = { path = "../trillium" }
trillium-native-tls = { path = "../native-tls" }
trillium-rustls = { path = "../rustls" }
trillium-smol = { path = "../smol/" }
trillium-testing = { path = "../testing" }

Expand Down
183 changes: 183 additions & 0 deletions client/examples/cli.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
use blocking::Unblock;
use clap::Parser;
use std::{
io::{ErrorKind, IsTerminal},
path::PathBuf,
str::FromStr,
};
use trillium::{Body, Method};
use trillium_client::{Client, Conn, Error, FollowRedirects};
use trillium_native_tls::NativeTlsConfig;
use trillium_rustls::RustlsConfig;
use trillium_smol::ClientConfig;
use url::{self, Url};

pub fn main() {
ClientCli::parse().run()
}

#[derive(Parser, Debug)]
pub struct ClientCli {
#[arg(value_parser = parse_method_case_insensitive)]
method: Method,

#[arg(value_parser = parse_url)]
url: Url,

/// provide a file system path to a file to use as the request body
///
/// alternatively, you can use an operating system pipe to pass a file in
///
/// three equivalent examples:
///
/// trillium client post http://httpbin.org/anything -f ./body.json
/// trillium client post http://httpbin.org/anything < ./body.json
/// cat ./body.json | trillium client post http://httpbin.org/anything
#[arg(short, long, verbatim_doc_comment)]
file: Option<PathBuf>,

/// provide a request body on the command line
///
/// example:
/// trillium client post http://httpbin.org/post -b '{"hello": "world"}'
#[arg(short, long, verbatim_doc_comment)]
body: Option<String>,

/// provide headers in the form -h KEY1=VALUE1 KEY2=VALUE2
///
/// example:
/// trillium client get http://httpbin.org/headers -H Accept=application/json Authorization="Basic u:p"
#[arg(short = 'H', long, value_parser = parse_header, verbatim_doc_comment)]
headers: Vec<(String, String)>,

/// tls implementation. options: rustls, native-tls, none
///
/// requests to https:// urls with `none` will fail
#[arg(short, long, default_value = "rustls", verbatim_doc_comment)]
tls: TlsType,

/// set the log level. add more flags for more verbosity
///
/// example:
/// trillium client get https://www.google.com -vvv # `trace` verbosity level
#[command(flatten)]
verbose: clap_verbosity_flag::Verbosity,
}

impl ClientCli {
async fn build(&self) -> Conn {
let client = match self.tls {
TlsType::None => Client::new(ClientConfig::default()),
TlsType::Rustls => Client::new(RustlsConfig::<ClientConfig>::default()),
TlsType::NativeTls => Client::new(NativeTlsConfig::<ClientConfig>::default()),
};

let client = client
.with_handler(FollowRedirects::new())
.with_default_pool();

let mut conn = client.build_conn(self.method, self.url.clone());
for (name, value) in &self.headers {
conn.request_headers().append(name.clone(), value.clone());
}

if let Some(path) = &self.file {
let metadata = async_fs::metadata(path)
.await
.unwrap_or_else(|e| panic!("could not read file {:?} ({})", path, e));

let file = async_fs::File::open(path)
.await
.unwrap_or_else(|e| panic!("could not read file {:?} ({})", path, e));

conn.with_body(Body::new_streaming(file, Some(metadata.len())))
} else if let Some(body) = &self.body {
conn.with_body(body.clone())
} else if !std::io::stdin().is_terminal() {
conn.with_body(Body::new_streaming(Unblock::new(std::io::stdin()), None))
} else {
conn
}
}

pub fn run(self) {
trillium_smol::async_global_executor::block_on(async move {
env_logger::Builder::new()
.filter_module("trillium_client", self.verbose.log_level_filter())
.init();

let mut conn = self.build().await;

if let Err(e) = (&mut conn).await {
match e {
Error::Io(io) if io.kind() == ErrorKind::ConnectionRefused => {
log::error!("could not reach {}", self.url)
}

_ => log::error!("protocol error:\n\n{}", e),
}

return;
}

if std::io::stdout().is_terminal() {
let body = conn.response_body().read_string().await.unwrap();

let _request_headers_as_string = format!("{:#?}", conn.request_headers());
let headers = conn.response_headers();
let _response_headers_as_string = format!("{:#?}", headers);
let _status_string = conn.status().unwrap().to_string();
println!("{conn:#?}");
println!("{body}");
} else {
futures_lite::io::copy(
&mut conn.response_body(),
&mut Unblock::new(std::io::stdout()),
)
.await
.unwrap();
}
});
}
}

#[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone)]
enum TlsType {
None,
Rustls,
NativeTls,
}

fn parse_method_case_insensitive(src: &str) -> Result<Method, String> {
src.to_uppercase()
.parse()
.map_err(|_| format!("unrecognized method {}", src))
}

fn parse_url(src: &str) -> Result<Url, url::ParseError> {
if src.starts_with("http") {
src.parse()
} else {
format!("http://{}", src).parse()
}
}

impl FromStr for TlsType {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match &*s.to_ascii_lowercase() {
"none" => Ok(Self::None),
"rustls" => Ok(Self::Rustls),
"native" | "native-tls" => Ok(Self::NativeTls),
_ => Err(format!("unrecognized tls {}", s)),
}
}
}

fn parse_header(s: &str) -> Result<(String, String), String> {
let pos = s
.find('=')
.ok_or_else(|| format!("invalid KEY=value: no `=` found in `{}`", s))?;
Ok((String::from(&s[..pos]), String::from(&s[pos + 1..])))
}
54 changes: 40 additions & 14 deletions client/src/client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::{ClientLike, Conn, Pool};
use crate::{client_handler::ClientHandler, ClientLike, Conn, Pool};
use arc_swap::ArcSwapOption;

use std::{convert::TryInto, fmt::Debug, sync::Arc};
use trillium_http::{transport::BoxedTransport, Method};
use trillium_server_common::{Connector, ObjectSafeConnector, Url};
Expand All @@ -9,10 +11,15 @@ A client contains a Config and an optional connection pool and builds
conns.

*/

#[derive(Clone, Debug)]
pub struct Client {
config: Arc<dyn ObjectSafeConnector>,
pool: Option<Pool<Origin, BoxedTransport>>,
pub struct Client(Arc<ClientInner>);

#[derive(Debug)]
pub struct ClientInner {
config: Box<dyn ObjectSafeConnector>,
pool: ArcSwapOption<Pool<Origin, BoxedTransport>>,
handler: ArcSwapOption<Box<dyn ClientHandler>>,
}

macro_rules! method {
Expand Down Expand Up @@ -59,10 +66,11 @@ assert_eq!(conn.url().to_string(), \"http://localhost:8080/some/route\");
impl Client {
/// builds a new client from this `Connector`
pub fn new(config: impl Connector) -> Self {
Self {
config: config.arced(),
pool: None,
}
Self(Arc::new(ClientInner {
config: config.boxed(),
pool: ArcSwapOption::empty(),
handler: ArcSwapOption::empty(),
}))
}

/**
Expand All @@ -78,8 +86,8 @@ impl Client {
.with_default_pool(); //<-
```
*/
pub fn with_default_pool(mut self) -> Self {
self.pool = Some(Pool::default());
pub fn with_default_pool(self) -> Self {
self.0.pool.store(Some(Arc::new(Pool::default())));
self
}

Expand Down Expand Up @@ -109,15 +117,16 @@ impl Client {
U: TryInto<Url>,
<U as TryInto<Url>>::Error: Debug,
{
let mut conn = Conn::new_with_config(
Arc::clone(&self.config),
let mut conn = Conn::new_with_client(
self.clone(),
method.try_into().unwrap(),
url.try_into().unwrap(),
);

if let Some(pool) = &self.pool {
if let Some(pool) = self.0.pool.load_full().as_deref() {
conn.set_pool(pool.clone());
}

conn
}

Expand All @@ -128,7 +137,7 @@ impl Client {
intermittently.
*/
pub fn clean_up_pool(&self) {
if let Some(pool) = &self.pool {
if let Some(pool) = &*self.0.pool.load() {
pool.cleanup();
}
}
Expand All @@ -138,6 +147,23 @@ impl Client {
method!(put, Put);
method!(delete, Delete);
method!(patch, Patch);

pub(crate) fn handler(&self) -> Option<Arc<Box<dyn ClientHandler>>> {
self.0.handler.load_full()
}
///
pub fn with_handler(mut self, handler: impl ClientHandler) -> Self {
self.set_handler(handler);
self
}
///
pub fn set_handler(&mut self, handler: impl ClientHandler) {
self.0.handler.store(Some(Arc::new(Box::new(handler))))
}

pub(crate) fn connector(&self) -> &dyn ObjectSafeConnector {
&self.0.config
}
}

impl<T: Connector> From<T> for Client {
Expand Down
Loading
Loading