remove ClonableService usage

CHANGES.md

@@ -1,6 +1,8 @@
 # Changes
 
-## 0.6.1 (2019-07-11)
+## 0.6.1 (2019-07-19)
+
+* remove ClonableService usage
 
 * added comprehensive tests for session workflow
 

Cargo.toml

@@ -42,7 +42,7 @@ time = "0.1.42"
 
 # actix web session
 actix-web = { version = "1.0.3", optional = true }
-actix-utils = { version = "0.4.2", optional = true }
+actix-utils = { version = "0.4.5", optional = true }
 actix-service = { version = "0.4.1", optional = true }
 actix-session = { version = "0.2.0", optional = true }
 rand = { version = "0.7.0", optional = true }
@@ -50,7 +50,7 @@ serde = { version = "1.0.94", optional = true , features = ["derive"]}
 serde_json = { version = "1.0.40", optional = true }
 env_logger = "0.6.2"
 actix-http-test = "0.2.2"
-actix-http = "0.2.5"
+actix-http = "0.2.6"
 
 [dev-dependencies]
 env_logger = "0.6"

src/session.rs

@@ -1,8 +1,9 @@
+use std::cell::RefCell;
 use std::{collections::HashMap, iter, rc::Rc};
+
 use actix::prelude::*;
 use actix_service::{Service, Transform};
 use actix_session::{Session, SessionStatus};
-use actix_utils::cloneable::CloneableService;
 use actix_web::cookie::{Cookie, CookieJar, Key, SameSite};
 use actix_web::dev::{ServiceRequest, ServiceResponse};
 use actix_web::http::header::{self, HeaderValue};
@@ -103,7 +104,7 @@ where
 
     fn new_transform(&self, service: S) -> Self::Future {
         ok(RedisSessionMiddleware {
-            service: CloneableService::new(service),
+            service: Rc::new(RefCell::new(service)),
             inner: self.0.clone(),
         })
     }
@@ -111,7 +112,7 @@ where
 
 /// Cookie session middleware
 pub struct RedisSessionMiddleware<S: 'static> {
-    service: CloneableService<S>,
+    service: Rc<RefCell<S>>,
     inner: Rc<Inner>,
 }
 
@@ -128,7 +129,7 @@ where
     type Future = Box<Future<Item = Self::Response, Error = Self::Error>>;
 
     fn poll_ready(&mut self) -> Poll<(), Self::Error> {
-        self.service.poll_ready()
+        self.service.borrow_mut().poll_ready()
     }
 
     fn call(&mut self, mut req: ServiceRequest) -> Self::Future {
@@ -145,53 +146,50 @@ where
 
             srv.call(req).and_then(move |mut res| {
                 match Session::get_changes(&mut res) {
-                    (SessionStatus::Unchanged, None) =>
+                    (SessionStatus::Unchanged, None) => {
-                        Either::A(Either::A(Either::A(ok(res)))),
+                        Either::A(Either::A(Either::A(ok(res))))
-                    (SessionStatus::Unchanged, Some(state)) =>
+                    }
-                        Either::A(Either::A(Either::B(
+                    (SessionStatus::Unchanged, Some(state)) => {
-                            if value.is_none(){  // implies the session is new
+                        Either::A(Either::A(Either::B(if value.is_none() {
-                                Either::A(
+                            // implies the session is new
-                                    inner.update(res, state, value)
+                            Either::A(inner.update(res, state, value))
-                                )
+                        } else {
-                            } else {
+                            Either::B(ok(res))
-                                Either::B(
+                        })))
-                                    ok(res)
+                    }
-                                )
+                    (SessionStatus::Changed, Some(state)) => {
-                            }
+                        Either::A(Either::B(Either::A(inner.update(res, state, value))))
-                        ))),
+                    }
-                    (SessionStatus::Changed, Some(state)) => 
-                        Either::A(Either::B(Either::A(inner.update(res, state, value)))),
                     (SessionStatus::Purged, Some(_)) => {
-                            if let Some(val) = value {
+                        if let Some(val) = value {
-                                Either::A(Either::B(Either::B(Either::A(
+                            Either::A(Either::B(Either::B(Either::A(
-                                        inner.clear_cache(val)
+                                inner.clear_cache(val).and_then(move |_| {
-                                            .and_then(move |_| 
+                                    match inner.remove_cookie(&mut res) {
-                                                match inner.remove_cookie(&mut res){
+                                        Ok(_) => Either::A(ok(res)),
-                                                    Ok(_) => Either::A(ok(res)),
+                                        Err(_err) => Either::B(err(
-                                                    Err(_err) => Either::B(err(
+                                            error::ErrorInternalServerError(_err),
-                                                        error::ErrorInternalServerError(_err)))
+                                        )),
-                                            })
+                                    }
-                                ))))
+                                }),
-                            } else {
+                            ))))
-                                Either::A(Either::B(Either::B(Either::B(
+                        } else {
-                                    err(error::ErrorInternalServerError("unexpected"))
+                            Either::A(Either::B(Either::B(Either::B(err(
-                                ))))
+                                error::ErrorInternalServerError("unexpected"),
-                            }
+                            )))))
-                    },
+                        }
-                    (SessionStatus::Renewed, Some(state)) => { 
+                    }
+                    (SessionStatus::Renewed, Some(state)) => {
                         if let Some(val) = value {
                             Either::B(Either::A(
-                                inner.clear_cache(val)
+                                inner
-                                     .and_then(move |_|
+                                    .clear_cache(val)
-                                        inner.update(res, state, None))
+                                    .and_then(move |_| inner.update(res, state, None)),
                             ))
                         } else {
-                            Either::B(Either::B(
+                            Either::B(Either::B(inner.update(res, state, None)))
-                                inner.update(res, state, None)
-                            ))
                         }
-                    },
+                    }
-                    (_, None) => unreachable!()
+                    (_, None) => unreachable!(),
                 }
             })
         }))
@@ -329,97 +327,102 @@ impl Inner {
         }
     }
 
-    /// removes cache entry 
+    /// removes cache entry
-    fn clear_cache(&self, key: String)
+    fn clear_cache(&self, key: String) -> impl Future<Item = (), Error = Error> {
-                    -> impl Future<Item=(), Error=Error> {
         self.addr
-        .send(Command(resp_array!["DEL", key]))
+            .send(Command(resp_array!["DEL", key]))
-        .map_err(Error::from)
+            .map_err(Error::from)
-        .and_then(|res| {
+            .and_then(|res| {
-            match res {
+                match res {
-                // redis responds with number of deleted records
+                    // redis responds with number of deleted records
-                Ok(RespValue::Integer(x)) if x > 0 => Ok(()),
+                    Ok(RespValue::Integer(x)) if x > 0 => Ok(()),
-                _ => Err(error::ErrorInternalServerError(
+                    _ => Err(error::ErrorInternalServerError(
-                    "failed to remove session from cache"))
+                        "failed to remove session from cache",
-            }
+                    )),
-        })
+                }
-    } 
+            })
+    }
 
     /// invalidates session cookie
-    fn remove_cookie<B>(&self, res: &mut ServiceResponse<B>)
+    fn remove_cookie<B>(&self, res: &mut ServiceResponse<B>) -> Result<(), Error> {
-                    -> Result<(), Error> {
         let mut cookie = Cookie::named(self.name.clone());
         cookie.set_value("");
         cookie.set_max_age(Duration::seconds(0));
         cookie.set_expires(time::now() - Duration::days(365));
 
         let val = HeaderValue::from_str(&cookie.to_string())
-                    .map_err(|err| error::ErrorInternalServerError(err))?;
+            .map_err(|err| error::ErrorInternalServerError(err))?;
         res.headers_mut().append(header::SET_COOKIE, val);
 
         Ok(())
     }
 }
 
-
 #[cfg(test)]
 mod test {
     use super::*;
-    use actix_http::{HttpService, httpmessage::HttpMessage};
+    use actix_http::{httpmessage::HttpMessage, HttpService};
-    use actix_http_test::{TestServer, block_on};
+    use actix_http_test::{block_on, TestServer};
     use actix_session::Session;
-    use actix_web::{middleware, web, App, HttpResponse, HttpServer, Result,
+    use actix_web::{
-                    web::{resource, get, post}};
+        middleware, web,
+        web::{get, post, resource},
+        App, HttpResponse, HttpServer, Result,
+    };
     use serde::{Deserialize, Serialize};
     use serde_json::json;
     use time;
 
-
     #[derive(Serialize, Deserialize, Debug, PartialEq)]
     pub struct IndexResponse {
         user_id: Option<String>,
-        counter: i32
+        counter: i32,
     }
 
     fn index(session: Session) -> Result<HttpResponse> {
         let user_id: Option<String> = session.get::<String>("user_id").unwrap();
-        let counter: i32 = session.get::<i32>("counter")
+        let counter: i32 = session
-                                  .unwrap_or(Some(0))
+            .get::<i32>("counter")
-                                  .unwrap_or(0);
+            .unwrap_or(Some(0))
+            .unwrap_or(0);
 
-        Ok(HttpResponse::Ok().json(IndexResponse{user_id, counter}))
+        Ok(HttpResponse::Ok().json(IndexResponse { user_id, counter }))
     }
 
-
     fn do_something(session: Session) -> Result<HttpResponse> {
         let user_id: Option<String> = session.get::<String>("user_id").unwrap();
-        let counter: i32 = session.get::<i32>("counter")
+        let counter: i32 = session
-                                  .unwrap_or(Some(0))
+            .get::<i32>("counter")
-                                  .map_or(1, |inner| inner + 1);
+            .unwrap_or(Some(0))
+            .map_or(1, |inner| inner + 1);
         session.set("counter", counter)?;
 
-        Ok(HttpResponse::Ok().json(IndexResponse{user_id, counter}))
+        Ok(HttpResponse::Ok().json(IndexResponse { user_id, counter }))
     }
 
     #[derive(Deserialize)]
     struct Identity {
-        user_id: String
+        user_id: String,
     }
     fn login(user_id: web::Json<Identity>, session: Session) -> Result<HttpResponse> {
         let id = user_id.into_inner().user_id;
         session.set("user_id", &id)?;
         session.renew();
 
-        let counter: i32 = session.get::<i32>("counter")
+        let counter: i32 = session
-                                  .unwrap_or(Some(0))
+            .get::<i32>("counter")
-                                  .unwrap_or(0);
+            .unwrap_or(Some(0))
+            .unwrap_or(0);
 
-        Ok(HttpResponse::Ok().json(IndexResponse{user_id: Some(id), counter}))
+        Ok(HttpResponse::Ok().json(IndexResponse {
+            user_id: Some(id),
+            counter,
+        }))
     }
 
     fn logout(session: Session) -> Result<HttpResponse> {
         let id: Option<String> = session.get("user_id")?;
-        if let Some(x) = id{
+        if let Some(x) = id {
             session.purge();
             Ok(format!("Logged out: {}", x).into())
         } else {
@@ -429,17 +432,17 @@ mod test {
 
     #[test]
     fn test_workflow() {
-        // Step 1:  GET index 
+        // Step 1:  GET index
         //   - set-cookie actix-session will be in response (session cookie #1)
         //   - response should be: {"counter": 0, "user_id": None}
         // Step 2:  GET index, including session cookie #1 in request
         //   - set-cookie will *not* be in response
         //   - response should be: {"counter": 0, "user_id": None}
         // Step 3: POST to do_something, including session cookie #1 in request
-        //   - adds new session state in redis:  {"counter": 1} 
+        //   - adds new session state in redis:  {"counter": 1}
         //   - response should be: {"counter": 1, "user_id": None}
         // Step 4: POST again to do_something, including session cookie #1 in request
-        //   - updates session state in redis:  {"counter": 2} 
+        //   - updates session state in redis:  {"counter": 2}
         //   - response should be: {"counter": 2, "user_id": None}
         // Step 5: POST to login, including session cookie #1 in request
         //   - set-cookie actix-session will be in response  (session cookie #2)
@@ -459,86 +462,124 @@ mod test {
         //   - set-cookie actix-session will be in response (session cookie #3)
         //   - response should be: {"counter": 0, "user_id": None}
 
-        let mut srv =
+        let mut srv = TestServer::new(|| {
-            TestServer::new(|| {
+            HttpService::new(
-                HttpService::new(
+                App::new()
-                    App::new()
+                    .wrap(
-                        .wrap(RedisSession::new("127.0.0.1:6379", &[0; 32])
+                        RedisSession::new("127.0.0.1:6379", &[0; 32])
-                                            .cookie_name("test-session"))
+                            .cookie_name("test-session"),
-                        .wrap(middleware::Logger::default())
+                    )
-                        .service(resource("/").route(get().to(index)))
+                    .wrap(middleware::Logger::default())
-                        .service(resource("/do_something").route(post().to(do_something)))
+                    .service(resource("/").route(get().to(index)))
-                        .service(resource("/login").route(post().to(login)))
+                    .service(resource("/do_something").route(post().to(do_something)))
-                        .service(resource("/logout").route(post().to(logout)))
+                    .service(resource("/login").route(post().to(login)))
-                )
+                    .service(resource("/logout").route(post().to(logout))),
-            });
+            )
+        });
 
-        
+        // Step 1:  GET index
-        // Step 1:  GET index  
         //   - set-cookie actix-session will be in response (session cookie #1)
         //   - response should be: {"counter": 0, "user_id": None}
         let req_1a = srv.get("/").send();
         let mut resp_1 = srv.block_on(req_1a).unwrap();
-        let cookie_1 = resp_1.cookies().unwrap().clone()
+        let cookie_1 = resp_1
-                             .into_iter().find(|c| c.name() == "test-session")
+            .cookies()
-                             .unwrap();
+            .unwrap()
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session")
+            .unwrap();
         let result_1 = block_on(resp_1.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_1, IndexResponse{user_id: None, counter: 0});
+        assert_eq!(
-
+            result_1,
+            IndexResponse {
+                user_id: None,
+                counter: 0
+            }
+        );
 
         // Step 2:  GET index, including session cookie #1 in request
         //   - set-cookie will *not* be in response
         //   - response should be: {"counter": 0, "user_id": None}
         let req_2 = srv.get("/").cookie(cookie_1.clone()).send();
         let resp_2 = srv.block_on(req_2).unwrap();
-        let cookie_2 = resp_2.cookies().unwrap().clone()
+        let cookie_2 = resp_2
-                             .into_iter().find(|c| c.name() == "test-session");
+            .cookies()
+            .unwrap()
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session");
         assert_eq!(cookie_2, None);
 
-
         // Step 3: POST to do_something, including session cookie #1 in request
-        //   - adds new session state in redis:  {"counter": 1} 
+        //   - adds new session state in redis:  {"counter": 1}
         //   - response should be: {"counter": 1, "user_id": None}
         let req_3 = srv.post("/do_something").cookie(cookie_1.clone()).send();
         let mut resp_3 = srv.block_on(req_3).unwrap();
         let result_3 = block_on(resp_3.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_3, IndexResponse{user_id: None, counter: 1});
+        assert_eq!(
-
+            result_3,
+            IndexResponse {
+                user_id: None,
+                counter: 1
+            }
+        );
 
         // Step 4: POST again to do_something, including session cookie #1 in request
-        //   - updates session state in redis:  {"counter": 2} 
+        //   - updates session state in redis:  {"counter": 2}
         //   - response should be: {"counter": 2, "user_id": None}
         let req_4 = srv.post("/do_something").cookie(cookie_1.clone()).send();
         let mut resp_4 = srv.block_on(req_4).unwrap();
         let result_4 = block_on(resp_4.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_4, IndexResponse{user_id: None, counter: 2});
+        assert_eq!(
-
+            result_4,
+            IndexResponse {
+                user_id: None,
+                counter: 2
+            }
+        );
 
         // Step 5: POST to login, including session cookie #1 in request
         //   - set-cookie actix-session will be in response  (session cookie #2)
         //   - updates session state in redis: {"counter": 2, "user_id": "ferris"}
-        let req_5 = srv.post("/login")
+        let req_5 = srv
-                       .cookie(cookie_1.clone())
+            .post("/login")
-                       .send_json(&json!({"user_id": "ferris"}));
+            .cookie(cookie_1.clone())
+            .send_json(&json!({"user_id": "ferris"}));
         let mut resp_5 = srv.block_on(req_5).unwrap();
-        let cookie_2 = resp_5.cookies().unwrap().clone()
+        let cookie_2 = resp_5
-                             .into_iter().find(|c| c.name() == "test-session")
+            .cookies()
-                             .unwrap();
+            .unwrap()
-        assert_eq!(true, cookie_1.value().to_string() != cookie_2.value().to_string());
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session")
+            .unwrap();
+        assert_eq!(
+            true,
+            cookie_1.value().to_string() != cookie_2.value().to_string()
+        );
 
         let result_5 = block_on(resp_5.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_5, IndexResponse{user_id: Some("ferris".into()), counter: 2});
+        assert_eq!(
-
+            result_5,
+            IndexResponse {
+                user_id: Some("ferris".into()),
+                counter: 2
+            }
+        );
 
         // Step 6: GET index, including session cookie #2 in request
         //   - response should be: {"counter": 2, "user_id": "ferris"}
-        let req_6 = srv.get("/")
+        let req_6 = srv.get("/").cookie(cookie_2.clone()).send();
-                       .cookie(cookie_2.clone())
-                       .send();
         let mut resp_6 = srv.block_on(req_6).unwrap();
         let result_6 = block_on(resp_6.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_6, IndexResponse{user_id: Some("ferris".into()), counter: 2});
+        assert_eq!(
-
+            result_6,
+            IndexResponse {
+                user_id: Some("ferris".into()),
+                counter: 2
+            }
+        );
 
         // Step 7: POST again to do_something, including session cookie #2 in request
         //   - updates session state in redis: {"counter": 3, "user_id": "ferris"}
@@ -546,50 +587,71 @@ mod test {
         let req_7 = srv.post("/do_something").cookie(cookie_2.clone()).send();
         let mut resp_7 = srv.block_on(req_7).unwrap();
         let result_7 = block_on(resp_7.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_7, IndexResponse{user_id: Some("ferris".into()), counter: 3});
+        assert_eq!(
-
+            result_7,
+            IndexResponse {
+                user_id: Some("ferris".into()),
+                counter: 3
+            }
+        );
 
         // Step 8: GET index, including session cookie #1 in request
         //   - set-cookie actix-session will be in response (session cookie #3)
         //   - response should be: {"counter": 0, "user_id": None}
-        let req_8 = srv.get("/")
+        let req_8 = srv.get("/").cookie(cookie_1.clone()).send();
-                       .cookie(cookie_1.clone())
-                       .send();
         let mut resp_8 = srv.block_on(req_8).unwrap();
-        let cookie_3 = resp_8.cookies().unwrap().clone()
+        let cookie_3 = resp_8
-                             .into_iter().find(|c| c.name() == "test-session")
+            .cookies()
-                             .unwrap();
+            .unwrap()
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session")
+            .unwrap();
         let result_8 = block_on(resp_8.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_8, IndexResponse{user_id: None, counter: 0});
+        assert_eq!(
+            result_8,
+            IndexResponse {
+                user_id: None,
+                counter: 0
+            }
+        );
         assert!(cookie_3.value().to_string() != cookie_2.value().to_string());
 
-
         // Step 9: POST to logout, including session cookie #2
         //   - set-cookie actix-session will be in response with session cookie #2
         //     invalidation logic
-        let req_9 = srv.post("/logout")
+        let req_9 = srv.post("/logout").cookie(cookie_2.clone()).send();
-                       .cookie(cookie_2.clone())
-                       .send();
         let resp_9 = srv.block_on(req_9).unwrap();
-        let cookie_4 = resp_9.cookies().unwrap().clone()
+        let cookie_4 = resp_9
-                             .into_iter().find(|c| c.name() == "test-session")
+            .cookies()
-                             .unwrap();
+            .unwrap()
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session")
+            .unwrap();
         assert!(&time::now().tm_year != &cookie_4.expires().map(|t| t.tm_year).unwrap());
 
-
         // Step 10: GET index, including session cookie #2 in request
         //   - set-cookie actix-session will be in response (session cookie #3)
         //   - response should be: {"counter": 0, "user_id": None}
-        let req_10 = srv.get("/")
+        let req_10 = srv.get("/").cookie(cookie_2.clone()).send();
-                        .cookie(cookie_2.clone())
-                        .send();
         let mut resp_10 = srv.block_on(req_10).unwrap();
         let result_10 = block_on(resp_10.json::<IndexResponse>()).unwrap();
-        assert_eq!(result_10, IndexResponse{user_id: None, counter: 0});
+        assert_eq!(
-        
+            result_10,
-        let cookie_5 = resp_10.cookies().unwrap().clone()
+            IndexResponse {
-                             .into_iter().find(|c| c.name() == "test-session")
+                user_id: None,
-                             .unwrap();
+                counter: 0
+            }
+        );
+
+        let cookie_5 = resp_10
+            .cookies()
+            .unwrap()
+            .clone()
+            .into_iter()
+            .find(|c| c.name() == "test-session")
+            .unwrap();
         assert!(cookie_5.value().to_string() != cookie_2.value().to_string());
     }
 }