From 093d3a6c59338045f46ee85f1a3ec3b028f97712 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 27 Dec 2020 23:23:30 +0000 Subject: [PATCH] remove deprecated on_connect methods (#1857) --- actix-http/CHANGES.md | 7 +++++++ actix-http/src/builder.rs | 23 ----------------------- actix-http/src/clinu/mod.rs | 0 actix-http/src/h1/dispatcher.rs | 18 ------------------ actix-http/src/h1/service.rs | 23 ----------------------- actix-http/src/h2/dispatcher.rs | 10 ---------- actix-http/src/h2/service.rs | 25 ------------------------- actix-http/src/helpers.rs | 14 -------------- actix-http/src/service.rs | 30 +----------------------------- actix-http/tests/test_openssl.rs | 2 -- actix-http/tests/test_server.rs | 2 -- 11 files changed, 8 insertions(+), 146 deletions(-) create mode 100644 actix-http/src/clinu/mod.rs diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 81577688d..c43246bb0 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -4,6 +4,13 @@ ### Changed * Bumped `rand` to `0.8` +### Removed +* Deprecated `on_connect` methods have been removed. Prefer the new + `on_connect_ext` technique. [#1857] + +[#1857]: https://github.com/actix/actix-web/pull/1857 + + ## 2.2.0 - 2020-11-25 ### Added * HttpResponse builders for 1xx status codes. [#1768] diff --git a/actix-http/src/builder.rs b/actix-http/src/builder.rs index b28c69761..df1b332c1 100644 --- a/actix-http/src/builder.rs +++ b/actix-http/src/builder.rs @@ -10,7 +10,6 @@ use crate::config::{KeepAlive, ServiceConfig}; use crate::error::Error; use crate::h1::{Codec, ExpectHandler, H1Service, UpgradeHandler}; use crate::h2::H2Service; -use crate::helpers::{Data, DataFactory}; use crate::request::Request; use crate::response::Response; use crate::service::HttpService; @@ -28,8 +27,6 @@ pub struct HttpServiceBuilder> { local_addr: Option, expect: X, upgrade: Option, - // DEPRECATED: in favor of on_connect_ext - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, S)>, } @@ -51,7 +48,6 @@ where local_addr: None, expect: ExpectHandler, upgrade: None, - on_connect: None, on_connect_ext: None, _t: PhantomData, } @@ -141,7 +137,6 @@ where local_addr: self.local_addr, expect: expect.into_factory(), upgrade: self.upgrade, - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } @@ -171,26 +166,11 @@ where local_addr: self.local_addr, expect: self.expect, upgrade: Some(upgrade.into_factory()), - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } } - /// Set on-connect callback. - /// - /// Called once per connection. Return value of the call is stored in request extensions. - /// - /// *SOFT DEPRECATED*: Prefer the `on_connect_ext` style callback. - pub fn on_connect(mut self, f: F) -> Self - where - F: Fn(&T) -> I + 'static, - I: Clone + 'static, - { - self.on_connect = Some(Rc::new(move |io| Box::new(Data(f(io))))); - self - } - /// Sets the callback to be run on connection establishment. /// /// Has mutable access to a data container that will be merged into request extensions. @@ -224,7 +204,6 @@ where H1Service::with_config(cfg, service.into_factory()) .expect(self.expect) .upgrade(self.upgrade) - .on_connect(self.on_connect) .on_connect_ext(self.on_connect_ext) } @@ -247,7 +226,6 @@ where ); H2Service::with_config(cfg, service.into_factory()) - .on_connect(self.on_connect) .on_connect_ext(self.on_connect_ext) } @@ -272,7 +250,6 @@ where HttpService::with_config(cfg, service.into_factory()) .expect(self.expect) .upgrade(self.upgrade) - .on_connect(self.on_connect) .on_connect_ext(self.on_connect_ext) } } diff --git a/actix-http/src/clinu/mod.rs b/actix-http/src/clinu/mod.rs new file mode 100644 index 000000000..e69de29bb diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index ea8f91e0d..91e208aac 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -19,7 +19,6 @@ use crate::cloneable::CloneableService; use crate::config::ServiceConfig; use crate::error::{DispatchError, Error}; use crate::error::{ParseError, PayloadError}; -use crate::helpers::DataFactory; use crate::httpmessage::HttpMessage; use crate::request::Request; use crate::response::Response; @@ -96,7 +95,6 @@ where service: CloneableService, expect: CloneableService, upgrade: Option>, - on_connect: Option>, on_connect_data: Extensions, flags: Flags, peer_addr: Option, @@ -184,7 +182,6 @@ where service: CloneableService, expect: CloneableService, upgrade: Option>, - on_connect: Option>, on_connect_data: Extensions, peer_addr: Option, ) -> Self { @@ -197,7 +194,6 @@ where service, expect, upgrade, - on_connect, on_connect_data, peer_addr, ) @@ -213,7 +209,6 @@ where service: CloneableService, expect: CloneableService, upgrade: Option>, - on_connect: Option>, on_connect_data: Extensions, peer_addr: Option, ) -> Self { @@ -246,7 +241,6 @@ where service, expect, upgrade, - on_connect, on_connect_data, flags, peer_addr, @@ -572,12 +566,6 @@ where let pl = this.codec.message_type(); req.head_mut().peer_addr = *this.peer_addr; - // DEPRECATED - // set on_connect data - if let Some(ref on_connect) = this.on_connect { - on_connect.set(&mut req.extensions_mut()); - } - // merge on_connect_ext data into request extensions req.extensions_mut().drain_from(this.on_connect_data); @@ -1038,7 +1026,6 @@ mod tests { CloneableService::new(ok_service()), CloneableService::new(ExpectHandler), None, - None, Extensions::new(), None, ); @@ -1079,7 +1066,6 @@ mod tests { CloneableService::new(echo_path_service()), CloneableService::new(ExpectHandler), None, - None, Extensions::new(), None, ); @@ -1134,7 +1120,6 @@ mod tests { CloneableService::new(echo_path_service()), CloneableService::new(ExpectHandler), None, - None, Extensions::new(), None, ); @@ -1184,7 +1169,6 @@ mod tests { CloneableService::new(echo_payload_service()), CloneableService::new(ExpectHandler), None, - None, Extensions::new(), None, ); @@ -1256,7 +1240,6 @@ mod tests { CloneableService::new(echo_path_service()), CloneableService::new(ExpectHandler), None, - None, Extensions::new(), None, ); @@ -1316,7 +1299,6 @@ mod tests { CloneableService::new(ok_service()), CloneableService::new(ExpectHandler), Some(CloneableService::new(UpgradeHandler(PhantomData))), - None, Extensions::new(), None, ); diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index cbba0609c..919a5d932 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -15,7 +15,6 @@ use crate::body::MessageBody; use crate::cloneable::CloneableService; use crate::config::ServiceConfig; use crate::error::{DispatchError, Error}; -use crate::helpers::DataFactory; use crate::request::Request; use crate::response::Response; use crate::{ConnectCallback, Extensions}; @@ -30,7 +29,6 @@ pub struct H1Service> { cfg: ServiceConfig, expect: X, upgrade: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B)>, } @@ -53,7 +51,6 @@ where srv: service.into_factory(), expect: ExpectHandler, upgrade: None, - on_connect: None, on_connect_ext: None, _t: PhantomData, } @@ -215,7 +212,6 @@ where cfg: self.cfg, srv: self.srv, upgrade: self.upgrade, - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } @@ -232,21 +228,11 @@ where cfg: self.cfg, srv: self.srv, expect: self.expect, - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } } - /// Set on connect callback. - pub(crate) fn on_connect( - mut self, - f: Option Box>>, - ) -> Self { - self.on_connect = f; - self - } - /// Set on connect callback. pub(crate) fn on_connect_ext(mut self, f: Option>>) -> Self { self.on_connect_ext = f; @@ -284,7 +270,6 @@ where fut_upg: self.upgrade.as_ref().map(|f| f.new_service(())), expect: None, upgrade: None, - on_connect: self.on_connect.clone(), on_connect_ext: self.on_connect_ext.clone(), cfg: Some(self.cfg.clone()), _t: PhantomData, @@ -314,7 +299,6 @@ where fut_upg: Option, expect: Option, upgrade: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, cfg: Option, _t: PhantomData<(T, B)>, @@ -371,7 +355,6 @@ where service, this.expect.take().unwrap(), this.upgrade.take(), - this.on_connect.clone(), this.on_connect_ext.clone(), ) })) @@ -383,7 +366,6 @@ pub struct H1ServiceHandler { srv: CloneableService, expect: CloneableService, upgrade: Option>, - on_connect: Option Box>>, on_connect_ext: Option>>, cfg: ServiceConfig, _t: PhantomData<(T, B)>, @@ -405,7 +387,6 @@ where srv: S, expect: X, upgrade: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, ) -> H1ServiceHandler { H1ServiceHandler { @@ -413,7 +394,6 @@ where expect: CloneableService::new(expect), upgrade: upgrade.map(CloneableService::new), cfg, - on_connect, on_connect_ext, _t: PhantomData, } @@ -480,8 +460,6 @@ where } fn call(&mut self, (io, addr): Self::Request) -> Self::Future { - let deprecated_on_connect = self.on_connect.as_ref().map(|handler| handler(&io)); - let mut connect_extensions = Extensions::new(); if let Some(ref handler) = self.on_connect_ext { // run on_connect_ext callback, populating connect extensions @@ -494,7 +472,6 @@ where self.srv.clone(), self.expect.clone(), self.upgrade.clone(), - deprecated_on_connect, connect_extensions, addr, ) diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index 594339121..7a0be9492 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -18,7 +18,6 @@ use crate::body::{BodySize, MessageBody, ResponseBody}; use crate::cloneable::CloneableService; use crate::config::ServiceConfig; use crate::error::{DispatchError, Error}; -use crate::helpers::DataFactory; use crate::httpmessage::HttpMessage; use crate::message::ResponseHead; use crate::payload::Payload; @@ -36,7 +35,6 @@ where { service: CloneableService, connection: Connection, - on_connect: Option>, on_connect_data: Extensions, config: ServiceConfig, peer_addr: Option, @@ -57,7 +55,6 @@ where pub(crate) fn new( service: CloneableService, connection: Connection, - on_connect: Option>, on_connect_data: Extensions, config: ServiceConfig, timeout: Option, @@ -84,7 +81,6 @@ where config, peer_addr, connection, - on_connect, on_connect_data, ka_expire, ka_timer, @@ -134,12 +130,6 @@ where head.headers = parts.headers.into(); head.peer_addr = this.peer_addr; - // DEPRECATED - // set on_connect data - if let Some(ref on_connect) = this.on_connect { - on_connect.set(&mut req.extensions_mut()); - } - // merge on_connect_ext data into request extensions req.extensions_mut().drain_from(&mut this.on_connect_data); diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index 3103127b4..b1fb9a634 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -20,7 +20,6 @@ use crate::body::MessageBody; use crate::cloneable::CloneableService; use crate::config::ServiceConfig; use crate::error::{DispatchError, Error}; -use crate::helpers::DataFactory; use crate::request::Request; use crate::response::Response; use crate::{ConnectCallback, Extensions}; @@ -31,7 +30,6 @@ use super::dispatcher::Dispatcher; pub struct H2Service { srv: S, cfg: ServiceConfig, - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B)>, } @@ -51,23 +49,12 @@ where ) -> Self { H2Service { cfg, - on_connect: None, on_connect_ext: None, srv: service.into_factory(), _t: PhantomData, } } - /// Set on connect callback. - - pub(crate) fn on_connect( - mut self, - f: Option Box>>, - ) -> Self { - self.on_connect = f; - self - } - /// Set on connect callback. pub(crate) fn on_connect_ext(mut self, f: Option>>) -> Self { self.on_connect_ext = f; @@ -212,7 +199,6 @@ where H2ServiceResponse { fut: self.srv.new_service(()), cfg: Some(self.cfg.clone()), - on_connect: self.on_connect.clone(), on_connect_ext: self.on_connect_ext.clone(), _t: PhantomData, } @@ -225,7 +211,6 @@ pub struct H2ServiceResponse { #[pin] fut: S::Future, cfg: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B)>, } @@ -248,7 +233,6 @@ where let this = self.as_mut().project(); H2ServiceHandler::new( this.cfg.take().unwrap(), - this.on_connect.clone(), this.on_connect_ext.clone(), service, ) @@ -260,7 +244,6 @@ where pub struct H2ServiceHandler { srv: CloneableService, cfg: ServiceConfig, - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B)>, } @@ -275,13 +258,11 @@ where { fn new( cfg: ServiceConfig, - on_connect: Option Box>>, on_connect_ext: Option>>, srv: S, ) -> H2ServiceHandler { H2ServiceHandler { cfg, - on_connect, on_connect_ext, srv: CloneableService::new(srv), _t: PhantomData, @@ -312,8 +293,6 @@ where } fn call(&mut self, (io, addr): Self::Request) -> Self::Future { - let deprecated_on_connect = self.on_connect.as_ref().map(|handler| handler(&io)); - let mut connect_extensions = Extensions::new(); if let Some(ref handler) = self.on_connect_ext { // run on_connect_ext callback, populating connect extensions @@ -325,7 +304,6 @@ where Some(self.srv.clone()), Some(self.cfg.clone()), addr, - deprecated_on_connect, Some(connect_extensions), server::handshake(io), ), @@ -343,7 +321,6 @@ where Option>, Option, Option, - Option>, Option, Handshake, ), @@ -379,7 +356,6 @@ where ref mut srv, ref mut config, ref peer_addr, - ref mut on_connect, ref mut on_connect_data, ref mut handshake, ) => match Pin::new(handshake).poll(cx) { @@ -387,7 +363,6 @@ where self.state = State::Incoming(Dispatcher::new( srv.take().unwrap(), conn, - on_connect.take(), on_connect_data.take().unwrap(), config.take().unwrap(), None, diff --git a/actix-http/src/helpers.rs b/actix-http/src/helpers.rs index ac0e0f118..13195f7db 100644 --- a/actix-http/src/helpers.rs +++ b/actix-http/src/helpers.rs @@ -3,8 +3,6 @@ use std::io; use bytes::{BufMut, BytesMut}; use http::Version; -use crate::extensions::Extensions; - const DIGITS_START: u8 = b'0'; pub(crate) fn write_status_line(version: Version, n: u16, bytes: &mut BytesMut) { @@ -56,18 +54,6 @@ impl<'a> io::Write for Writer<'a> { } } -pub(crate) trait DataFactory { - fn set(&self, ext: &mut Extensions); -} - -pub(crate) struct Data(pub(crate) T); - -impl DataFactory for Data { - fn set(&self, ext: &mut Extensions) { - ext.insert(self.0.clone()) - } -} - #[cfg(test)] mod tests { use std::str::from_utf8; diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 75745209c..527ed3833 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -17,7 +17,6 @@ use crate::builder::HttpServiceBuilder; use crate::cloneable::CloneableService; use crate::config::{KeepAlive, ServiceConfig}; use crate::error::{DispatchError, Error}; -use crate::helpers::DataFactory; use crate::request::Request; use crate::response::Response; use crate::{h1, h2::Dispatcher, ConnectCallback, Extensions, Protocol}; @@ -28,8 +27,6 @@ pub struct HttpService cfg: ServiceConfig, expect: X, upgrade: Option, - // DEPRECATED: in favor of on_connect_ext - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B)>, } @@ -67,7 +64,6 @@ where srv: service.into_factory(), expect: h1::ExpectHandler, upgrade: None, - on_connect: None, on_connect_ext: None, _t: PhantomData, } @@ -83,7 +79,6 @@ where srv: service.into_factory(), expect: h1::ExpectHandler, upgrade: None, - on_connect: None, on_connect_ext: None, _t: PhantomData, } @@ -116,7 +111,6 @@ where cfg: self.cfg, srv: self.srv, upgrade: self.upgrade, - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } @@ -142,21 +136,11 @@ where cfg: self.cfg, srv: self.srv, expect: self.expect, - on_connect: self.on_connect, on_connect_ext: self.on_connect_ext, _t: PhantomData, } } - /// Set on connect callback. - pub(crate) fn on_connect( - mut self, - f: Option Box>>, - ) -> Self { - self.on_connect = f; - self - } - /// Set connect callback with mutable access to request data container. pub(crate) fn on_connect_ext(mut self, f: Option>>) -> Self { self.on_connect_ext = f; @@ -366,7 +350,6 @@ where fut_upg: self.upgrade.as_ref().map(|f| f.new_service(())), expect: None, upgrade: None, - on_connect: self.on_connect.clone(), on_connect_ext: self.on_connect_ext.clone(), cfg: self.cfg.clone(), _t: PhantomData, @@ -391,7 +374,6 @@ pub struct HttpServiceResponse< fut_upg: Option, expect: Option, upgrade: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, cfg: ServiceConfig, _t: PhantomData<(T, B)>, @@ -451,7 +433,6 @@ where service, this.expect.take().unwrap(), this.upgrade.take(), - this.on_connect.clone(), this.on_connect_ext.clone(), ) })) @@ -464,7 +445,6 @@ pub struct HttpServiceHandler { expect: CloneableService, upgrade: Option>, cfg: ServiceConfig, - on_connect: Option Box>>, on_connect_ext: Option>>, _t: PhantomData<(T, B, X)>, } @@ -486,12 +466,10 @@ where srv: S, expect: X, upgrade: Option, - on_connect: Option Box>>, on_connect_ext: Option>>, ) -> HttpServiceHandler { HttpServiceHandler { cfg, - on_connect, on_connect_ext, srv: CloneableService::new(srv), expect: CloneableService::new(expect), @@ -564,7 +542,6 @@ where fn call(&mut self, (io, proto, peer_addr): Self::Request) -> Self::Future { let mut connect_extensions = Extensions::new(); - let deprecated_on_connect = self.on_connect.as_ref().map(|handler| handler(&io)); if let Some(ref handler) = self.on_connect_ext { handler(&io, &mut connect_extensions); } @@ -575,7 +552,6 @@ where server::handshake(io), self.cfg.clone(), self.srv.clone(), - deprecated_on_connect, connect_extensions, peer_addr, ))), @@ -588,7 +564,6 @@ where self.srv.clone(), self.expect.clone(), self.upgrade.clone(), - deprecated_on_connect, connect_extensions, peer_addr, )), @@ -617,7 +592,6 @@ where Handshake, ServiceConfig, CloneableService, - Option>, Extensions, Option, )>, @@ -694,12 +668,10 @@ where } else { panic!() }; - let (_, cfg, srv, on_connect, on_connect_data, peer_addr) = - data.take().unwrap(); + let (_, cfg, srv, on_connect_data, peer_addr) = data.take().unwrap(); self.set(State::H2(Dispatcher::new( srv, conn, - on_connect, on_connect_data, cfg, None, diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 05f01d240..6b80bad0a 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -410,10 +410,8 @@ async fn test_h2_service_error() { async fn test_h2_on_connect() { let srv = test_server(move || { HttpService::build() - .on_connect(|_| 10usize) .on_connect_ext(|_, data| data.insert(20isize)) .h2(|req: Request| { - assert!(req.extensions().contains::()); assert!(req.extensions().contains::()); ok::<_, ()>(Response::Ok().finish()) }) diff --git a/actix-http/tests/test_server.rs b/actix-http/tests/test_server.rs index de6368fda..44794e199 100644 --- a/actix-http/tests/test_server.rs +++ b/actix-http/tests/test_server.rs @@ -662,10 +662,8 @@ async fn test_h1_service_error() { async fn test_h1_on_connect() { let srv = test_server(|| { HttpService::build() - .on_connect(|_| 10usize) .on_connect_ext(|_, data| data.insert(20isize)) .h1(|req: Request| { - assert!(req.extensions().contains::()); assert!(req.extensions().contains::()); future::ok::<_, ()>(Response::Ok().finish()) })