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

Refactor middleware interface #17

Merged
merged 1 commit into from
Jun 2, 2016
Merged
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: 6 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
[package]

name = "nickel_postgres"
version = "0.0.0"
version = "0.2.0"
authors = [
"bguiz"
]

[dependencies]
postgres = "*"
r2d2 = "*"
r2d2_postgres = "*"
plugin = "*"
typemap = "*"
openssl = "*"
nickel = "*"
nickel = "0.8.1"
r2d2 = "0.7.0"
r2d2_postgres = "0.10.1"
typemap = "0.3.3"
plugin = "0.2.6"
17 changes: 5 additions & 12 deletions examples/example.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,19 @@
extern crate r2d2;
extern crate postgres;
extern crate openssl;
#[macro_use] extern crate nickel;
extern crate nickel_postgres;

use std::env;
use r2d2::NopErrorHandler;
use postgres::SslMode;
use nickel::{Nickel, HttpRouter};
use nickel_postgres::{PostgresMiddleware, PostgresRequestExtensions};

fn main() {
let mut app = Nickel::new();

let postgres_url = env::var("DATABASE_URL").unwrap();
let dbpool = PostgresMiddleware::new(&*postgres_url,
SslMode::None,
5,
Box::new(NopErrorHandler)).unwrap();
app.utilize(dbpool);
app.get("/my_counter", middleware! { |request|
let _connection = request.db_conn();
let mw = PostgresMiddleware::new(&postgres_url).unwrap();
app.utilize(mw);

app.get("/my_counter", middleware! { |request, response|
let _connection = try_with!(response, request.pg_conn());

// use connection
});
Expand Down
31 changes: 31 additions & 0 deletions examples/with_pool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
extern crate r2d2;
extern crate r2d2_postgres;
#[macro_use] extern crate nickel;
extern crate nickel_postgres;

use std::env;
use r2d2::{Config, Pool};
use r2d2_postgres::{PostgresConnectionManager, SslMode};
use nickel::{Nickel, HttpRouter};
use nickel_postgres::{PostgresMiddleware, PostgresRequestExtensions};

fn main() {
let mut app = Nickel::new();

let postgres_url = env::var("DATABASE_URL").unwrap();
let db_mgr = PostgresConnectionManager::new(postgres_url.as_ref(), SslMode::None)
.expect("Unable to connect to database");

let db_pool = Pool::new(Config::default(), db_mgr)
.expect("Unable to initialize connection pool");

app.utilize(PostgresMiddleware::with_pool(db_pool));

app.get("/my_counter", middleware! { |request, response|
let _connection = try_with!(response, request.pg_conn());

// use connection
});

app.get("**", middleware! { println!("!!!") });
}
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
extern crate nickel;
extern crate postgres;
extern crate r2d2;
extern crate r2d2_postgres;
extern crate plugin;
extern crate typemap;
extern crate plugin;

pub use middleware::{ PostgresMiddleware, PostgresRequestExtensions };

Expand Down
68 changes: 43 additions & 25 deletions src/middleware.rs
Original file line number Diff line number Diff line change
@@ -1,51 +1,69 @@
use std::sync::Arc;
use std::error::Error as StdError;

use std::error::Error;
use std::result::Result;
use nickel::{Request, Response, Middleware, Continue, MiddlewareResult};
use postgres::{SslMode};
use r2d2_postgres::{PostgresConnectionManager};
use r2d2::{Pool, HandleError, Config, PooledConnection};
use nickel::status::StatusCode;
use r2d2_postgres::{PostgresConnectionManager, SslMode};
use r2d2::{Config, Pool, PooledConnection, GetTimeout};
use typemap::Key;
use plugin::{Pluggable, Extensible};
use plugin::Extensible;

pub struct PostgresMiddleware {
pub pool: Arc<Pool<PostgresConnectionManager>>
pub pool: Pool<PostgresConnectionManager>,
}

impl PostgresMiddleware {
pub fn new(connect_str: &str,
ssl_mode: SslMode,
num_connections: u32,
error_handler: Box<HandleError<::r2d2_postgres::Error>>)
-> Result<PostgresMiddleware, Box<StdError>> {
let manager = try!(PostgresConnectionManager::new(connect_str, ssl_mode));

let config = Config::builder()
.pool_size(num_connections)
.error_handler(error_handler)
.build();
/// Create middleware using defaults
///
/// The middleware will be setup with no ssl and the r2d2 defaults.
pub fn new(db_url: &str) -> Result<PostgresMiddleware, Box<Error>> {
let manager = try!(PostgresConnectionManager::new(db_url, SslMode::None));
let pool = try!(Pool::new(Config::default(), manager));

let pool = try!(Pool::new(config, manager));
Ok(PostgresMiddleware { pool: pool })
}

Ok(PostgresMiddleware { pool: Arc::new(pool) })
/// Create middleware using pre-built `r2d2::Pool`
///
/// This allows the caller to create and configure the pool with specific settings.
pub fn with_pool(pool: Pool<PostgresConnectionManager>) -> PostgresMiddleware {
PostgresMiddleware { pool: pool }
}
}

impl Key for PostgresMiddleware { type Value = Arc<Pool<PostgresConnectionManager>>; }
impl Key for PostgresMiddleware { type Value = Pool<PostgresConnectionManager>; }

impl<D> Middleware<D> for PostgresMiddleware {
fn invoke<'mw, 'conn>(&self, req: &mut Request<'mw, 'conn, D>, res: Response<'mw, D>) -> MiddlewareResult<'mw, D> {
req.extensions_mut().insert::<PostgresMiddleware>(self.pool.clone());

Ok(Continue(res))
}
}

/// Add `pg_conn()` helper method to `nickel::Request`
///
/// This trait must only be used in conjunction with `PostgresMiddleware`.
///
/// On error, the method returns a tuple per Nickel convention. This allows the route to use the
/// `try_with!` macro.
///
/// Example:
///
/// ```ignore
/// app.get("/my_counter", middleware! { |request, response|
/// let db = try_with!(response, request.pg_conn());
/// });
/// ```
pub trait PostgresRequestExtensions {
fn db_conn(&self) -> PooledConnection<PostgresConnectionManager>;
fn pg_conn(&self) -> Result<PooledConnection<PostgresConnectionManager>, (StatusCode, GetTimeout)>;
}

impl<'a, 'b, D> PostgresRequestExtensions for Request<'a, 'b, D> {
fn db_conn(&self) -> PooledConnection<PostgresConnectionManager> {
self.extensions().get::<PostgresMiddleware>().unwrap().get().unwrap()
fn pg_conn(&self) -> Result<PooledConnection<PostgresConnectionManager>, (StatusCode, GetTimeout)> {
self.extensions()
.get::<PostgresMiddleware>()
.expect("PostgresMiddleware must be registered before using PostgresRequestExtensions::pg_conn()")
.get()
.or_else(|err| Err((StatusCode::InternalServerError, err)))
Copy link
Member

Choose a reason for hiding this comment

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

Can this only be triggered on a timeout? Would GatewayTimeout be appropriate? https://tools.ietf.org/html/rfc7231#section-6.6.5

Copy link
Contributor Author

@hjr3 hjr3 May 29, 2016

Choose a reason for hiding this comment

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

This is the only error possible. This error happens when a thread from Pool tries to create a new connection to Postgres. The method inside of r2d2 is called get, so the error that was chosen was GetTimeout.

I think GatewayTimeout is more appropriate when acting as a proxy. I think we should return a 500 or 503 in this case.

Copy link
Member

Choose a reason for hiding this comment

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

You're right, either one of the two you've suggested is fine I think 👍

}
}