4
\$\begingroup\$

Iron's cookie and session middleware projects seem to be abandoned. I've put together implementations for them, along with a session flash provider. They seem to be working fine. They are all very small, so I'll paste them in their entirety below (each file is linked to a Github repo and each repository also contains a working example). I would appreciate any input!

A couple specific questions/concerns:

  1. The library user brings their own session store and flash object implementations. I've never really programmed with generics before, so I don't know if any of this is reasonable. For example, I'm using an associated type for the flash object (which the user defines in implementing Flashable on their session object) rather than a trait type parameter. I think this is the right choice, but I mostly made it to keep the signatures shorter...
  2. In session_fe::Store::get, there is a possibly unnecessary clone. Would it be better to return an Option<Arc<T>> to remove the clone? That's definitely doable, but might be slightly more unwieldy if the user needs to mutate the object before setting it back. Also, would it be good to wrap the inner value in a RwLock so that it could be swapped out without write-locking the whole map?

cookie_fe/src/lib.rs

extern crate iron;
extern crate cookie;

use iron::prelude::*;
use iron::{AroundMiddleware, Handler, typemap};
use iron::headers::{Cookie, SetCookie};

pub use cookie::CookieJar;
pub use cookie::Cookie as CookiePair;

pub struct Builder(&'static [u8]);

impl Builder {
    pub fn new(key: &'static [u8]) -> Self { Builder(key) }
}

pub struct Util(&'static [u8], Option<CookieJar<'static>>);

impl Util {

    pub fn jar(&mut self) -> Option<&CookieJar<'static>> {
        if self.1.is_none() { 
            self.1 = Some(CookieJar::new(self.0));
        }
        self.1.as_ref()
    }

}

impl typemap::Key for Util { type Value = Self; }

impl AroundMiddleware for Builder {
    fn around(self, handler: Box<Handler>) -> Box<Handler> {
        let wrapper = Wrapper {
            builder: self,
            handler: handler
        };
        Box::new(wrapper) as Box<Handler>
    }
}


struct Wrapper<H: Handler> { 
    builder: Builder, 
    handler: H
}

impl<H: Handler> Handler for Wrapper<H> {
    fn handle(&self, req: &mut Request) -> IronResult<Response> {

        let jar = req.headers.get::<Cookie>()
            .map(|x| x.to_cookie_jar(self.builder.0) );

        let util = Util(self.builder.0, jar);

        req.extensions.insert::<Util>(util);

        let mut res = self.handler.handle(req);

        if let Ok(&mut ref mut r) = res.as_mut() {
            if let Some(jar) = req.extensions.get::<Util>()
                .and_then(|x| x.1.as_ref() ) {

                let delta = jar.delta();
                if !delta.is_empty() {
                    r.headers.set(SetCookie(delta));
                } 
            }                 
        }

        res
    }
}

session_fe/src/lib.rs

extern crate iron;

use std::collections::HashMap;

use std::sync::{Arc, RwLock};
use std::convert::Into;

use std::fmt::Debug;
use std::any::Any;

use iron::prelude::*;
use iron::{typemap, BeforeMiddleware};

impl<T: Clone + Debug + Any> typemap::Key for Util<T> { type Value = Util<T>; }

#[derive(Debug, Clone)]
struct Store<T: Clone + Debug>(Arc<RwLock<HashMap<String, T>>>);

impl<T: Clone + Debug> Store<T> {

    pub fn new() -> Self {
        Store(Arc::new(RwLock::new(HashMap::new())))
    }

    pub fn get(&self, key: &str) -> Option<T> {
        self.0.read().iter()
            .filter_map(|g| (*g).get(key) )
            .cloned()
            .next()
    }

    fn insert(&self, key: String, value: T) {
        if let Ok(mut lock) = self.0.write() {
            (*lock).insert(key, value);
        }
    }

    fn remove(&self, key: &str) {
        if let Ok(mut lock) = self.0.write() {
            (*lock).remove(key);
        }
    }

}

pub struct Builder<T: Clone + Debug> {
    key: Box<Fn(&mut Request) -> String + Send + Sync>,
    store: Store<T>
}

impl<T: Clone + Debug> Builder<T> {

    pub fn new(key: Box<Fn(&mut Request) -> String + Send + Sync>) -> Self {
        Builder { key: key, store: Store::new() }
    }

}

pub struct Util<T: Clone + Debug> {
    key: String,
    store: Store<T>
}

impl<T: Clone + Debug> Util<T> {

    pub fn get(&self) -> Option<T> {
        self.store.get(&self.key)
    }

    pub fn set(&self, value: T) {
        self.store.insert(self.key.clone(), value);
    }

    pub fn destroy(&self) {
        self.store.remove(&self.key);
    }

}

impl<T: Clone + Debug + Send + Sync + Any> BeforeMiddleware for Builder<T> {
    fn before(&self, req: &mut Request) -> IronResult<()> {
        let key = (self.key)(req);
        let util = Util { key: key, store: self.store.clone() };
        req.extensions.insert::<Util<T>>(util);
        Ok(())
    }
}

flash_fe/src/lib.rs

extern crate iron;
extern crate session_fe;

use std::fmt::Debug;
use std::any::Any;
use std::marker::PhantomData;


use iron::prelude::*;
use iron::{Handler, AroundMiddleware, typemap};

use session_fe::Util as SessionUtil;

#[derive(Clone, Debug)]
pub struct Util<T: Flashable + Debug + Clone + Any> {
    pub now: Option<T::Object>,
    pub next: Option<T::Object>,
    pub pd_type: PhantomData<T>
}

pub trait Flashable {
    type Object: Debug + Clone + Any;

    fn new() -> Self;
    fn flash(&self) -> Option<Self::Object>;
    fn set_flash(&mut self, val: Option<Self::Object>);
}

impl<T: Flashable + Debug + Clone + Any> Util<T> {

    pub fn new() -> Self {
        Util { 
            now: None,
            next: None,
            pd_type: PhantomData
        }
    }

    pub fn rotate_in(&mut self, req: &Request) {
        if let Some(obj) = req.extensions.get::<SessionUtil<T>>()
            .and_then(|s| s.get() ) {
            if let Some(flash) = obj.flash() {
                self.now = Some(flash);
            }
        }  
    }

    pub fn rotate_out(&self, req: &Request) {
        if let Some(sess) = req.extensions.get::<SessionUtil<T>>() {
            if let Some(ref next) = self.next {
                if let Some(mut obj) = sess.get() {
                    obj.set_flash(Some(next.clone()));
                    sess.set(obj);
                } else {
                    let mut obj = <T>::new();
                    obj.set_flash(Some(next.clone()));
                    sess.set(obj);
                }
            } else if let Some(mut obj) = sess.get() {
                obj.set_flash(None);
                sess.set(obj);
            }            
        }
    }

    pub fn get(&self) -> Option<T::Object> {
        self.now.clone()
    }

    pub fn set(&mut self, value: Option<T::Object>) {
        self.next = value;
    }

}

impl<T: Flashable + Debug + Clone + Any> typemap::Key for Util<T> { type Value = Self; }

pub struct Builder<T: Flashable + Debug + Clone + Any>(PhantomData<T>);

impl<T: Flashable + Debug + Clone + Any> Builder<T> {
    pub fn new() -> Self {
        Builder(PhantomData)
    }
}

struct Rotator<H: Handler, T: Flashable + Debug + Clone + Any> {
    handler: H,
    pd_type: PhantomData<T>
}

impl<H: Handler, T: Flashable + Debug + Clone + Any + Send + Sync> Handler for Rotator<H, T> {
    fn handle(&self, req: &mut Request) -> IronResult<Response> {

        let mut util = Util::new();

        util.rotate_in(req);

        req.extensions.insert::<Util<T>>(util);

        let res = self.handler.handle(req);

        if res.is_ok() {
            if let Some(util) = req.extensions.get::<Util<T>>() {              
                util.rotate_out(req);
            }
        }

        res
    }
}

impl<T: Flashable + Debug + Clone + Any + Send + Sync> AroundMiddleware for Builder<T> {
    fn around(self, handler: Box<Handler>) -> Box<Handler> {
        let rotator = Rotator {
            handler: handler,
            pd_type: self.0
        };
        Box::new(rotator) as Box<Handler>
    }
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

I've never used these libraries so I only have surface-level comments, but I'm answering anyway because I feel bad you've been ignored. Luckily, I think the lack of answers is at least partly because your code look totally fine. It's much easier to critique worse code.

In particular, though you've "never really programmed with generics before", you're pretty fluent with them. Your choices seem good to me.


Rustaceans don't tend to write things on one line like this

pub fn new(key: &'static [u8]) -> Self { Builder(key) }

Similarly some of your spacing elsewhere seems inconsistent or excessive. In particular, trailing spaces.

Box::new(wrapper) as Box<Handler>

is fine as

Box::new(wrapper)

and I'd make the whole function just

Box::new(Wrapper {
    builder: self,
    handler: handler
})

Traits are normally imported fully, so rather than

impl typemap::Key for Util

you'd import Key and impl Key for Util.


if let Ok(&mut ref mut r) = res.as_mut()

is just

if let Ok(r) = res.as_mut()

I don't like the name Builder; it's too generic. In fact, in your own usage you rename it:

use cookie_fe::{Builder as CookieBuilder, Util as CookieUtil, CookiePair};

Doesn't that say something?


For session-fe, use std::convert::Into; is unused. There's a warning there. Also, learn auto-deref.

(*g).get(key) → g.get(key)
(*lock).insert(key, value) → lock.insert(key, value)
(*lock).remove(key) → lock.remove(key)

Not using auto-deref looks really odd.


You can just

Store(Default::default())

for the Store.

Then the iter here is suspect

self.0.read().iter()
    .filter_map(|g| g.get(key))
    .cloned()
    .next()

Writing

self.0.read().ok()
    .and_then(|g| g.get(key).cloned())

is clearer. If you want to remove the cloned, you're going to have to "return" a lock as well, since the read is only valid while the lock is held. This might be a good idea, but it's really up to you if the cost is worth it.


<T>::new is just T::new, and I believe rotate_out can be simplified from

if let Some(sess) = req.extensions.get::<SessionUtil<T>>() {
    if let Some(ref next) = self.next {
        if let Some(mut obj) = sess.get() {
            obj.set_flash(Some(next.clone()));
            sess.set(obj);
        } else {
            let mut obj = <T>::new();
            obj.set_flash(Some(next.clone()));
            sess.set(obj);
        }
    } else if let Some(mut obj) = sess.get() {
        obj.set_flash(None);
        sess.set(obj);
    }
}

to

if let Some(sess) = req.extensions.get::<SessionUtil<T>>() {
    let set = |mut obj: T| {
        obj.set_flash(self.next.clone());
        sess.set(obj);
    };

    if let Some(obj) = sess.get() {
        set(obj);
    } else if self.next.is_some() {
        set(T::new());
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks! I'm laughing now at my &mut ref mut r binding and explicit derefs. It's funny the superstitions/weird habits we pick up with an incomplete understanding of a language. I appreciate you taking the time to write this up! \$\endgroup\$
    – kardeiz
    Commented Jul 25, 2016 at 14:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.