From 36429ab50c1c98dc5f036ba78d84f080e767132f Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 3 Dec 2014 16:41:27 -0800 Subject: [PATCH] refactor(net): NetworkConnecter no longer is for static usage Instead, you can use an instance of a NetworkConnector with `Request::with_connector`. This allows overloading of the NetworkStream constructors, so that it is easy to modify how an `HttpStream` is created, while still relying on the rest of the stream implementation. BREAKING CHANGE --- benches/client_mock_tcp.rs | 11 +++++++---- src/client/request.rs | 19 ++++++++++--------- src/http.rs | 25 +++++++++++++++++++++++++ src/mock.rs | 7 ++++--- src/net.rs | 34 ++++++++++++++-------------------- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/benches/client_mock_tcp.rs b/benches/client_mock_tcp.rs index 8fe47e9e..c6ffc42b 100644 --- a/benches/client_mock_tcp.rs +++ b/benches/client_mock_tcp.rs @@ -92,8 +92,10 @@ impl net::NetworkStream for MockStream { } } -impl net::NetworkConnector for MockStream { - fn connect(_addr: To, _scheme: &str) -> IoResult { +struct MockConnector; + +impl net::NetworkConnector for MockConnector { + fn connect(&mut self, _addr: To, _scheme: &str) -> IoResult { Ok(MockStream::new()) } @@ -103,8 +105,9 @@ impl net::NetworkConnector for MockStream { fn bench_mock_hyper(b: &mut test::Bencher) { let url = "http://127.0.0.1:1337/"; b.iter(|| { - let mut req = hyper::client::Request::with_stream::( - hyper::Get, hyper::Url::parse(url).unwrap()).unwrap(); + let mut req = hyper::client::Request::with_connector( + hyper::Get, hyper::Url::parse(url).unwrap(), &mut MockConnector + ).unwrap(); req.headers_mut().set(Foo); req diff --git a/src/client/request.rs b/src/client/request.rs index 634dcceb..045c5093 100644 --- a/src/client/request.rs +++ b/src/client/request.rs @@ -7,7 +7,7 @@ use method; use method::Method::{Get, Post, Delete, Put, Patch, Head, Options}; use header::Headers; use header::common::{mod, Host}; -use net::{NetworkStream, NetworkConnector, HttpStream, Fresh, Streaming}; +use net::{NetworkStream, NetworkConnector, HttpConnector, Fresh, Streaming}; use HttpError::HttpUriError; use http::{HttpWriter, LINE_ENDING}; use http::HttpWriter::{ThroughWriter, ChunkedWriter, SizedWriter, EmptyWriter}; @@ -42,11 +42,12 @@ impl Request { impl Request { /// Create a new client request. pub fn new(method: method::Method, url: Url) -> HttpResult> { - Request::with_stream::(method, url) + let mut conn = HttpConnector; + Request::with_connector(method, url, &mut conn) } /// Create a new client request with a specific underlying NetworkStream. - pub fn with_stream(method: method::Method, url: Url) -> HttpResult> { + pub fn with_connector, S: NetworkStream>(method: method::Method, url: Url, connector: &mut C) -> HttpResult> { debug!("{} {}", method, url); let host = match url.serialize_host() { Some(host) => host, @@ -59,7 +60,7 @@ impl Request { }; debug!("port={}", port); - let stream: S = try!(NetworkConnector::connect((host[], port), url.scheme.as_slice())); + let stream: S = try!(connector.connect((host[], port), &*url.scheme)); let stream = ThroughWriter(BufferedWriter::new(box stream as Box)); let mut headers = Headers::new(); @@ -210,13 +211,13 @@ mod tests { use std::str::from_utf8; use url::Url; use method::Method::{Get, Head}; - use mock::MockStream; + use mock::{MockStream, MockConnector}; use super::Request; #[test] fn test_get_empty_body() { - let req = Request::with_stream::( - Get, Url::parse("http://example.dom").unwrap() + let req = Request::with_connector( + Get, Url::parse("http://example.dom").unwrap(), &mut MockConnector ).unwrap(); let req = req.start().unwrap(); let stream = *req.body.end().unwrap().into_inner().downcast::().unwrap(); @@ -228,8 +229,8 @@ mod tests { #[test] fn test_head_empty_body() { - let req = Request::with_stream::( - Head, Url::parse("http://example.dom").unwrap() + let req = Request::with_connector( + Head, Url::parse("http://example.dom").unwrap(), &mut MockConnector ).unwrap(); let req = req.start().unwrap(); let stream = *req.body.end().unwrap().into_inner().downcast::().unwrap(); diff --git a/src/http.rs b/src/http.rs index a2e85c15..56c14780 100644 --- a/src/http.rs +++ b/src/http.rs @@ -190,6 +190,31 @@ impl HttpWriter { } } + /// Access the inner Writer. + #[inline] + pub fn get_ref<'a>(&'a self) -> &'a W { + match *self { + ThroughWriter(ref w) => w, + ChunkedWriter(ref w) => w, + SizedWriter(ref w, _) => w, + EmptyWriter(ref w) => w, + } + } + + /// Access the inner Writer mutably. + /// + /// Warning: You should not write to this directly, as you can corrupt + /// the state. + #[inline] + pub fn get_mut<'a>(&'a mut self) -> &'a mut W { + match *self { + ThroughWriter(ref mut w) => w, + ChunkedWriter(ref mut w) => w, + SizedWriter(ref mut w, _) => w, + EmptyWriter(ref mut w) => w, + } + } + /// Ends the HttpWriter, and returns the underlying Writer. /// /// A final `write()` is called with an empty message, and then flushed. diff --git a/src/mock.rs b/src/mock.rs index 54cb5250..0e650702 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -61,14 +61,15 @@ impl Writer for MockStream { } impl NetworkStream for MockStream { - fn peer_name(&mut self) -> IoResult { Ok(from_str("127.0.0.1:1337").unwrap()) } } -impl NetworkConnector for MockStream { - fn connect(_addr: To, _scheme: &str) -> IoResult { +pub struct MockConnector; + +impl NetworkConnector for MockConnector { + fn connect(&mut self, _addr: To, _scheme: &str) -> IoResult { Ok(MockStream::new()) } } diff --git a/src/net.rs b/src/net.rs index 63a4a7bc..0f2207f4 100644 --- a/src/net.rs +++ b/src/net.rs @@ -9,7 +9,6 @@ use std::io::net::ip::{SocketAddr, ToSocketAddr}; use std::io::net::tcp::{TcpStream, TcpListener, TcpAcceptor}; use std::mem::{mod, transmute, transmute_copy}; use std::raw::{mod, TraitObject}; -use std::sync::{Arc, Mutex}; use uany::UncheckedBoxAnyDowncast; use openssl::ssl::{SslStream, SslContext}; @@ -53,9 +52,9 @@ pub trait NetworkStream: Stream + Any + Clone + Send { } /// A connector creates a NetworkStream. -pub trait NetworkConnector: NetworkStream { +pub trait NetworkConnector { /// Connect to a remote address. - fn connect(addr: To, scheme: &str) -> IoResult; + fn connect(&mut self, addr: To, scheme: &str) -> IoResult; } impl fmt::Show for Box { @@ -189,11 +188,7 @@ pub enum HttpStream { /// A stream over the HTTP protocol. Http(TcpStream), /// A stream over the HTTP protocol, protected by SSL. - // You may be asking wtf an Arc and Mutex? That's because SslStream - // doesn't implement Clone, and we need Clone to use the stream for - // both the Request and Response. - // FIXME: https://github.com/sfackler/rust-openssl/issues/6 - Https(Arc>>, SocketAddr), + Https(SslStream), } impl Reader for HttpStream { @@ -201,7 +196,7 @@ impl Reader for HttpStream { fn read(&mut self, buf: &mut [u8]) -> IoResult { match *self { Http(ref mut inner) => inner.read(buf), - Https(ref mut inner, _) => inner.lock().read(buf) + Https(ref mut inner) => inner.read(buf) } } } @@ -211,30 +206,32 @@ impl Writer for HttpStream { fn write(&mut self, msg: &[u8]) -> IoResult<()> { match *self { Http(ref mut inner) => inner.write(msg), - Https(ref mut inner, _) => inner.lock().write(msg) + Https(ref mut inner) => inner.write(msg) } } #[inline] fn flush(&mut self) -> IoResult<()> { match *self { Http(ref mut inner) => inner.flush(), - Https(ref mut inner, _) => inner.lock().flush(), + Https(ref mut inner) => inner.flush(), } } } - impl NetworkStream for HttpStream { fn peer_name(&mut self) -> IoResult { match *self { Http(ref mut inner) => inner.peer_name(), - Https(_, addr) => Ok(addr) + Https(ref mut inner) => inner.get_mut().peer_name() } } } -impl NetworkConnector for HttpStream { - fn connect(addr: To, scheme: &str) -> IoResult { +/// A connector that will produce HttpStreams. +pub struct HttpConnector; + +impl NetworkConnector for HttpConnector { + fn connect(&mut self, addr: To, scheme: &str) -> IoResult { match scheme { "http" => { debug!("http scheme"); @@ -242,13 +239,10 @@ impl NetworkConnector for HttpStream { }, "https" => { debug!("https scheme"); - let mut stream = try!(TcpStream::connect(addr)); - // we can't access the tcp stream once it's wrapped in an - // SslStream, so grab the ip address now, just in case. - let peer_addr = try!(stream.peer_name()); + let stream = try!(TcpStream::connect(addr)); let context = try!(SslContext::new(Sslv23).map_err(lift_ssl_error)); let stream = try!(SslStream::new(&context, stream).map_err(lift_ssl_error)); - Ok(Https(Arc::new(Mutex::new(stream)), peer_addr)) + Ok(Https(stream)) }, _ => { Err(IoError {