From 10e8d1898d03ec99f1c7bd456b79334c361e799d Mon Sep 17 00:00:00 2001 From: Patrick Donelan Date: Fri, 9 Apr 2010 01:12:30 -0400 Subject: [PATCH] More refactoring and documentation improvements --- README | 15 ++++++++- TODO | 6 +++- app.psgi | 24 +++++++++----- benchmark.pl | 7 ++-- eg/README | 23 +++++++++++++ lib/WebGUI.pm | 56 ++++++++++++++------------------ lib/WebGUI/Middleware/Session.pm | 50 ++++++++++++++++++++++++++++ lib/WebGUI/Session.pm | 6 ++-- 8 files changed, 140 insertions(+), 47 deletions(-) create mode 100644 eg/README create mode 100644 lib/WebGUI/Middleware/Session.pm diff --git a/README b/README index 14daa6d51..1854a0459 100644 --- a/README +++ b/README @@ -8,4 +8,17 @@ You can benchmark your server via: ab -t 3 -c 10 -k http://dev.localhost.localdomain:5000/ | grep Req -I'm currently getting 20 requests/second, whereas I'm getting 30/second on the non-PSGI WebGUI8 branch. \ No newline at end of file +I'm currently getting 23 requests/second, whereas I'm getting 30/second on the non-PSGI WebGUI8 branch. + += ARCHITECTURE = + +* The .psgi file gets to set WEBGUI_ROOT and WEBGUI_CONFIG. +* It instantiates the $wg WebGUI object (one per app). +* $wg creates and stores the WebGUI::Config (one per app) +* $wg creates the $app PSGI app code ref (one per app) +* WebGUI::Middleware::Session is wrapped around $app at the outer-most layer so that it can open and + close the $session WebGUI::Session. Any other wG middleware that needs $session should go in between + it and $app ($session created one per request) +* $session creates the $request WebGUI::Session::Request and $response WebGUI::Session::Response + objects (one per request) + \ No newline at end of file diff --git a/TODO b/TODO index 4981c66fe..de30c8e4d 100644 --- a/TODO +++ b/TODO @@ -7,4 +7,8 @@ DONE * serverObject gone from WebGUI::Session::open() * WebGUI::authen API changed * urlHandler API changed - no longer gets server, config -* Streaming response body \ No newline at end of file +* Streaming response body + +NB +* Periodically do a big stress-test and check for leaks, mysql overload etc.. + ab -t 100 -c 10 -k http://dev.localhost.localdomain:5000 | grep 'Req' \ No newline at end of file diff --git a/app.psgi b/app.psgi index 6306850cc..e36b92f3b 100644 --- a/app.psgi +++ b/app.psgi @@ -1,12 +1,18 @@ +use strict; +use Plack::Builder; use lib '/data/WebGUI/lib'; use WebGUI; -# Some ways to achieve the same thing from the command line: -# plackup -MWebGUI -e 'WebGUI->new' -# plackup -MWebGUI -e 'WebGUI->new("dev.localhost.localdomain.conf")' -# plackup -MWebGUI -e 'WebGUI->new(root => "/data/WebGUI", site => "dev.localhost.localdomain.conf")' -# -# Or from a .psgi file: -# my $app = WebGUI->new( root => '/data/WebGUI', site => 'dev.localhost.localdomain.conf' )->psgi_app; -# Or equivalently (using the defaults): -WebGUI->new; \ No newline at end of file +my $wg = WebGUI->new( root => '/data/WebGUI', site => 'dev.localhost.localdomain.conf' ); + +builder { + + # Open/close the WebGUI::Session at the outer-most onion layer + enable '+WebGUI::Middleware::Session', config => $wg->config; + + # Any additional WebGUI Middleware goes here + # .. + + # Return the app + $wg; +}; diff --git a/benchmark.pl b/benchmark.pl index 329a95c7b..e269ba619 100755 --- a/benchmark.pl +++ b/benchmark.pl @@ -1,14 +1,17 @@ # Little script used to run benchmarks against dev.localhost.localdomain # # To profile, run "perl -d:NYTProf benchmark.pl" +use Devel::Leak::Object qw(GLOBAL_bless); +$Devel::Leak::Object::TRACKSOURCELINES = 1; use lib '/data/WebGUI/lib'; use WebGUI; use Plack::Test; use HTTP::Request::Common; -my $app = WebGUI->new( root => '/data/WebGUI', site => 'dev.localhost.localdomain.conf' )->psgi_app; +my $wg = WebGUI->new( root => '/data/WebGUI', site => 'dev.localhost.localdomain.conf' ); +my $app = $wg->psgi_app; test_psgi $app, sub { my $cb = shift; my $res = $cb->( GET "/" ); -} for 1..100; +} for 1..100; \ No newline at end of file diff --git a/eg/README b/eg/README new file mode 100644 index 000000000..8b195c8f0 --- /dev/null +++ b/eg/README @@ -0,0 +1,23 @@ +# Some ways to achieve the same thing from the command line: +# plackup -MWebGUI -e 'WebGUI->new' +# plackup -MWebGUI -e 'WebGUI->new("dev.localhost.localdomain.conf")' +# plackup -MWebGUI -e 'WebGUI->new(root => "/data/WebGUI", site => "dev.localhost.localdomain.conf")' +# +# Or from a .psgi file: +# my $app = WebGUI->new( root => '/data/WebGUI', site => 'dev.localhost.localdomain.conf' )->psgi_app; + + + + # Extras + my $extrasURL = $wg->config->get('extrasURL'); + my $extrasPath = $wg->config->get('extrasPath'); + enable 'Plack::Middleware::Static', + path => sub { s{^$extrasURL/}{} }, + root => "$extrasPath/"; + + # Uploads + my $uploadsURL = $wg->config->get('uploadsURL'); + my $uploadsPath = $wg->config->get('uploadsPath'); + enable 'Plack::Middleware::Static', + path => sub { s{^$uploadsURL/}{} }, + root => "$uploadsPath/"; \ No newline at end of file diff --git a/lib/WebGUI.pm b/lib/WebGUI.pm index c8697e678..445643c70 100644 --- a/lib/WebGUI.pm +++ b/lib/WebGUI.pm @@ -29,12 +29,6 @@ use WebGUI::Session::Request; use Moose; use Try::Tiny; -has root => ( is => 'ro', isa => 'Str', default => '/data/WebGUI' ); -has site => ( is => 'ro', isa => 'Str', default => 'dev.localhost.localdomain.conf' ); -has config => ( is => 'rw', isa => 'WebGUI::Config' ); - -use overload q(&{}) => sub { shift->psgi_app }, fallback => 1; - =head1 NAME Package WebGUI @@ -53,6 +47,10 @@ These subroutines are available from this package: =cut +has root => ( is => 'ro', isa => 'Str', default => '/data/WebGUI' ); +has site => ( is => 'ro', isa => 'Str', default => 'dev.localhost.localdomain.conf' ); +has config => ( is => 'rw', isa => 'WebGUI::Config' ); + around BUILDARGS => sub { my $orig = shift; my $class = shift; @@ -74,7 +72,9 @@ sub BUILD { # Instantiate the WebGUI::Config object my $config = WebGUI::Config->new( $self->root, $self->site ); $self->config($config); -} +} + +use overload q(&{}) => sub { shift->psgi_app }, fallback => 1; sub psgi_app { my $self = shift; @@ -84,7 +84,7 @@ sub psgi_app { sub compile_psgi_app { my $self = shift; - my $catch = [ 500, [ 'Content-Type' => 'text/plain' ], [ "Internal Server Error\n" ] ]; + my $catch = [ 500, [ 'Content-Type' => 'text/plain' ], [ "Internal Server Error" ] ]; # WebGUI is a PSGI app is a Perl code reference. Let's create one. # Each web request results in a call to this sub @@ -96,10 +96,7 @@ sub compile_psgi_app { # unbuffered response writing return sub { my $responder = shift; - - # Open the WebGUI Session - # my $session = WebGUI::Session->open($self->root, $self->config, $env, $env->{'psgix.session'}->id); - my $session = WebGUI::Session->open($self->root, $self->config, $env); + my $session = $env->{'webgui.session'} or die 'Missing WebGUI Session - check WebGUI::Middleware::Session'; # Handle the request $self->handle($session); @@ -143,21 +140,22 @@ sub compile_psgi_app { $responder->( $catch ); } - } finally { - $session->close; - - }; + } } else { # Not streaming, so immediately tell the callback to return # the response. In the future we could use an Event framework here # to make this a non-blocking delayed response. - $session->close; $responder->($psgi_response); } } }; + # Wrap $app with some extra middleware that acts as a fallback for when + # you're not using something fast to serve static content + # + # This could also be in the .psgi file, but it seems sensible to have it + # baked in as a fallback (unless we find it drains performance) my $config = $self->config; # Extras @@ -176,12 +174,6 @@ sub compile_psgi_app { path => sub { s{^$uploadsURL/}{} }, root => "$uploadsPath/", ); - - # Session - TODO: make this user configurable - # use Plack::Middleware::Session; - # $app = Plack::Middleware::Session->wrap($app); - - return $app; } sub handle { @@ -199,16 +191,18 @@ sub handle { # This is generally a good thing to do, unless you want to send a file. # uncomment the following to short-circuit contentHandlers with a streaming response: -# $session->response->stream(sub { -# my $session = shift; -# $session->output->print("WebGUI PSGI with contentHandlers short-circuited for benchmarking (streaming)\n"); -# sleep 1; -# $session->output->print("...see?\n"); -# }); -# return; + # $session->response->stream( + # sub { + # my $session = shift; + # $session->output->print("WebGUI PSGI with contentHandlers short-circuited for benchmarking (streaming)\n"); + # #sleep 1; + # $session->output->print("...see?\n"); + # } + # ); + # return; # TODO: refactor the following loop, find all instances of "chunked" and "empty" in codebase, etc.. - for my $handler (@{$self->config->get("contentHandlers")}) { + for my $handler (@{$session->config->get("contentHandlers")}) { my $output = eval { WebGUI::Pluggable::run($handler, "handler", [ $session ] )}; if ( my $e = WebGUI::Error->caught ) { $session->errorHandler->error($e->package.":".$e->line." - ".$e->error); diff --git a/lib/WebGUI/Middleware/Session.pm b/lib/WebGUI/Middleware/Session.pm new file mode 100644 index 000000000..ce48b38e7 --- /dev/null +++ b/lib/WebGUI/Middleware/Session.pm @@ -0,0 +1,50 @@ +package WebGUI::Middleware::Session; +use strict; +use parent qw(Plack::Middleware); +use WebGUI::Config; +use WebGUI::Session; + +use Plack::Util::Accessor qw( config ); + +=head1 NAME + +WebGUI::Middleware::Session - Opens and closes the per-request WebGUI::Session + +=head1 DESCRIPTION + +This is PSGI middleware for WebGUI that instantiates, opens and closes the +L object. It does this as early and as late as possible, so +that all intermediate middleware (and the WebGUI app itself) can grab +the session out of the PSGI env hash: + + $env->{'webgui.session'}; + +and not worry about closing it. + +=cut + +sub call { + my ( $self, $env ) = @_; + + my $config = $self->config or die 'Mandatory config parameter missing'; + + # Open the Session + $env->{'webgui.session'} = WebGUI::Session->open( $config->getWebguiRoot, $config, $env ); + + # Run the app + my $res = $self->app->($env); + + # Use callback style response + return $self->response_cb( + $res, + sub { + my $res = shift; + + # Close the Session + $env->{'webgui.session'}->close(); + delete $env->{'webgui.session'}; + } + ); +} + +1; diff --git a/lib/WebGUI/Session.pm b/lib/WebGUI/Session.pm index d60538350..e27423320 100644 --- a/lib/WebGUI/Session.pm +++ b/lib/WebGUI/Session.pm @@ -145,7 +145,7 @@ sub close { # Kill circular references. The literal list is so that the order # can be explicitly shuffled as necessary. - foreach my $key (qw/_asset _datetime _icon _slave _db _env _form _http _id _output _os _privilege _scratch _setting _stow _style _url _user _var _cache _errorHandler/) { + foreach my $key (qw/_asset _datetime _icon _slave _db _env _form _http _id _output _os _privilege _scratch _setting _stow _style _url _user _var _cache _errorHandler _response _request/) { delete $self->{$key}; } } @@ -463,8 +463,8 @@ sub open { $self->{_request} = $request; $self->{_response} = $request->new_response( 200 ); - # TODO: it might be nice to set a default Content-type here, but we can't until Assets can override it again - # $self->{_response} = $request->new_response( 200 );#, [ 'Content-type' => 'text/html; charset=UTF-8' ] ); + # TODO: it might be nice to set a default Content-Type here, but we can't until Assets can override it again + # $self->{_response} = $request->new_response( 200 );#, [ 'Content-Type' => 'text/html; charset=UTF-8' ] ); # Use the WebGUI::Session::Request object to look up the sessionId from cookies, if it # wasn't given explicitly