From af705232a84b67d66edf5f2497dfbd9f6af94e79 Mon Sep 17 00:00:00 2001 From: JT Smith Date: Fri, 25 Sep 2009 18:41:03 -0500 Subject: [PATCH] added exceptions, docs, and removed disableCache --- etc/WebGUI.conf.original | 12 +++ lib/WebGUI/AssetLineage.pm | 6 +- lib/WebGUI/Cache.pm | 183 +++++++++++++++++++++++++++++-------- lib/WebGUI/Exception.pm | 122 ++++++++++++------------- lib/WebGUI/Session.pm | 1 - lib/WebGUI/Session/Stow.pm | 3 - t/Cache.t | 4 +- 7 files changed, 224 insertions(+), 107 deletions(-) diff --git a/etc/WebGUI.conf.original b/etc/WebGUI.conf.original index 34e395040..a3ba08a9f 100644 --- a/etc/WebGUI.conf.original +++ b/etc/WebGUI.conf.original @@ -88,6 +88,18 @@ #"webServerPort" : 80, +# The cacheServers directive tells WebGUI how to connect to +# memcached. If a "socket" is specified it will be used instead +# of the host and port info, and this should be the +# path to the unix socket that you started memcached with. "host" +# and "port" are used to tell WebGUI how to connect to +# memcached over TCP. And since this is an array you can specify +# as many server connections as you have memcached servers + + "cacheServers" : [ + { "socket" : "/tmp/memcached.sock", "host" : "127.0.0.1", "port" : "11211" } +], + # The database connection string. It usually takes the form of # DBI::;host: diff --git a/lib/WebGUI/AssetLineage.pm b/lib/WebGUI/AssetLineage.pm index 659b1d9f1..3a948c69d 100644 --- a/lib/WebGUI/AssetLineage.pm +++ b/lib/WebGUI/AssetLineage.pm @@ -265,10 +265,8 @@ sub getFirstChild { my $lineage = $assetLineage->{firstChild}{$self->getId}; unless ($lineage) { ($lineage) = $self->session->db->quickArray("select min(asset.lineage) from asset,assetData where asset.parentId=? and asset.assetId=assetData.assetId and asset.state='published'",[$self->getId]); - unless ($self->session->config->get("disableCache")) { - $assetLineage->{firstChild}{$self->getId} = $lineage; - $self->session->stow->set("assetLineage", $assetLineage); - } + $assetLineage->{firstChild}{$self->getId} = $lineage; + $self->session->stow->set("assetLineage", $assetLineage); } $child = WebGUI::Asset->newByLineage($self->session,$lineage); $self->cacheChild(first => $child); diff --git a/lib/WebGUI/Cache.pm b/lib/WebGUI/Cache.pm index 70a63b3a5..4fecdfe74 100644 --- a/lib/WebGUI/Cache.pm +++ b/lib/WebGUI/Cache.pm @@ -21,7 +21,7 @@ use HTTP::Request; use LWP::UserAgent; use Memcached::libmemcached; use Storable (); -use WebGUI::Error; +use WebGUI::Exception; use Params::Validate qw(:all); Params::Validate::validation_options( on_fail => sub { WebGUI::Error::InvalidParam->throw( error => shift ) } ); @@ -71,14 +71,29 @@ The key to delete. =cut sub delete { - validate(@_, - { name => { - type => SCALAR - } - }); - - my ($self, $name) = @_; - Memcached::libmemcached::memcached_delete($self->getMemcached, $self->parseKey($name)); + my ($self, $name) = validate_pos(@_, + 1, + { type => SCALAR | ARRAYREF }, + ); + my $memcached = $self->getMemcached; + Memcached::libmemcached::memcached_delete($memcached, $self->parseKey($name)); + if ($memcached->errstr eq 'SYSTEM ERROR Unknown error: 0') { + WebGUI::Error::Connection->throw( + error => "Cannot connect to memcached server." + ); + } + elsif ($memcached->errstr eq 'NO SERVERS DEFINED') { + WebGUI::Error->throw( + error => "No memcached servers specified in config file." + ); + } + elsif ($memcached->errstr ne 'SUCCESS' # deleted + && $memcached->errstr ne 'PROTOCOL ERROR' # doesn't exist to delete + ) { + WebGUI::Error->throw( + error => "Couldn't delete $name from cache because ".$memcached->errstr + ); + } } #------------------------------------------------------------------- @@ -91,7 +106,23 @@ Empties the caching system. sub flush { my ($self) = @_; - Memcached::libmemcached::memcached_flush($self->getMemcached); + my $memcached = $self->getMemcached; + Memcached::libmemcached::memcached_flush($memcached); + if ($memcached->errstr eq 'SYSTEM ERROR Unknown error: 0') { + WebGUI::Error::Connection->throw( + error => "Cannot connect to memcached server." + ); + } + elsif ($memcached->errstr eq 'NO SERVERS DEFINED') { + WebGUI::Error->throw( + error => "No memcached servers specified in config file." + ); + } + elsif ($memcached->errstr ne 'SUCCESS') { + WebGUI::Error->throw( + error => "Couldn't flush cache because ".$memcached->errstr + ); + } } #------------------------------------------------------------------- @@ -107,10 +138,35 @@ The key to retrieve. =cut sub get { - my ($self, $name) = @_; - my $content = Memcached::libmemcached::memcached_get($self->getMemcached, $self->parseKey($name)); + my ($self, $name) = validate_pos(@_, + 1, + { type => SCALAR | ARRAYREF }, + ); + my $memcached = $self->getMemcached; + my $content = Memcached::libmemcached::memcached_get($memcached, $self->parseKey($name)); + if ($memcached->errstr eq 'NOT FOUND' ) { + WebGUI::Error::ObjectNotFound->throw( + error => "The cache key $name has no value.", + id => $name, + ); + } + elsif ($memcached->errstr eq 'NO SERVERS DEFINED') { + WebGUI::Error->throw( + error => "No memcached servers specified in config file." + ); + } + elsif ($memcached->errstr eq 'SYSTEM ERROR Unknown error: 0') { + WebGUI::Error::Connection->throw( + error => "Cannot connect to memcached server." + ); + } + elsif ($memcached->errstr ne 'SUCCESS') { + WebGUI::Error->throw( + error => "Couldn't get $name from cache because ".$memcached->errstr + ); + } $content = Storable::thaw($content); - return undef unless $content && ref $content; + return undef unless ref $content; return ${$content}; } @@ -141,10 +197,25 @@ An array reference of keys to retrieve. =cut sub mget { - my ($self, $names) = @_; + my ($self, $names) = validate_pos(@_, + 1, + { type => ARRAYREF }, + ); my @parsedNames = map { $self->parseKey($_) } @{ $names }; my %result; - $self->getMemcached->mget_into_hashref(\@parsedNames, \%result); + my $memcached = $self->getMemcached; + $memcached->mget_into_hashref(\@parsedNames, \%result); + if ($memcached->errstr eq 'SYSTEM ERROR Unknown error: 0') { + WebGUI::Error::Connection->throw( + error => "Cannot connect to memcached server." + ); + } + elsif ($memcached->errstr eq 'NO SERVERS DEFINED') { + WebGUI::Error->throw( + error => "No memcached servers specified in config file." + ); + } + # no other useful status messages are returned my @values; foreach my $name (@parsedNames) { my $content = Storable::thaw($result{$name}); @@ -156,7 +227,7 @@ sub mget { #------------------------------------------------------------------- -=head2 new ( session, [ namespace ] ) +=head2 new ( session ) The new method will return a handler for the configured caching mechanism. Defaults to WebGUI::Cache::FileCache. You must override this method when building your own cache plug-in. @@ -164,23 +235,23 @@ The new method will return a handler for the configured caching mechanism. Defa A reference to the current session. -=head3 namespace - -A subdivider to store this cache under. When building your own cache plug-in default this to the WebGUI config file. - =cut sub new { - my ($class, $session, $namespace) = @_; + my ($class, $session) = validate_pos(@_, + 1, + { isa => 'WebGUI::Session' }, + ); + my ($class, $session) = @_; my $config = $session->config; - $namespace ||= $config->getFilename; - my $memcached = Memcached::libmemcached::memcached_create(); + my $namespace = $config->getFilename; + my $memcached = Memcached::libmemcached::memcached_create(); # no exception because always returns success foreach my $server (@{$config->get('cacheServers')}) { if (exists $server->{socket}) { - Memcached::libmemcached::memcached_server_add_unix_socket($memcached, $server->{socket}); + Memcached::libmemcached::memcached_server_add_unix_socket($memcached, $server->{socket}); # no exception because always returns success } else { - Memcached::libmemcached::memcached_server_add($memcached, $server->{host}, $server->{port}); + Memcached::libmemcached::memcached_server_add($memcached, $server->{host}, $server->{port}); # no exception because always returns success } } bless {_memcached => $memcached, _namespace => $namespace, _sesssion => $session}, $class; @@ -199,16 +270,16 @@ Can either be a text key, or a composite key. If it's a composite key, it will b =cut sub parseKey { - my ($self, $name) = @_; + my ($self, $name) = validate_pos(@_, + 1, + { type => SCALAR | ARRAYREF }, + ); # prepend namespace to the key my @key = ($self->{_namespace}); # check for composite or simple key, make array from either - if (! $name) { - # throw exception because no key was specified - } - elsif (ref $name eq 'ARRAY') { + if (ref $name eq 'ARRAY') { push @key, @{ $name }; } else { @@ -253,10 +324,30 @@ A time in seconds for the cache to exist. When you override default it to 60 sec =cut sub set { - my ($self, $name, $value, $ttl) = @_; - $ttl ||= 60; + my ($self, $name, $value, $ttl) = validate_pos(@_, + 1, + { type => SCALAR | ARRAYREF }, + { type => SCALAR }, + { type => SCALAR | UNDEF, optional => 1, default=> 60 }, + ); my $frozenValue = Storable::nfreeze(\(scalar $value)); # Storable doesn't like non-reference arguments, so we wrap it in a scalar ref. - Memcached::libmemcached::memcached_set($self->getMemcached, $self->parseKey($name), $frozenValue, $ttl); + my $memcached = $self->getMemcached; + Memcached::libmemcached::memcached_set($memcached, $self->parseKey($name), $frozenValue, $ttl); + if ($memcached->errstr eq 'SYSTEM ERROR Unknown error: 0') { + WebGUI::Error::Connection->throw( + error => "Cannot connect to memcached server." + ); + } + elsif ($memcached->errstr eq 'NO SERVERS DEFINED') { + WebGUI::Error->throw( + error => "No memcached servers specified in config file." + ); + } + elsif ($memcached->errstr ne 'SUCCESS') { + WebGUI::Error->throw( + error => "Couldn't set $name to cache because ".$memcached->errstr + ); + } return $value; } @@ -282,22 +373,42 @@ The time to live for this content. This is the amount of time (in seconds) that =cut sub setByHttp { - my ($self, $name, $url, $ttl) = @_; + my ($self, $name, $url, $ttl) = validate_pos(@_, + 1, + { type => SCALAR | ARRAYREF }, + { type => SCALAR }, + { type => SCALAR, optional => 1 }, + ); my $userAgent = new LWP::UserAgent; $userAgent->env_proxy; $userAgent->agent("WebGUI/".$WebGUI::VERSION); $userAgent->timeout(30); my $request = HTTP::Request->new(GET => $url); + + my $response = $userAgent->request($request); if ($response->is_error) { $self->session->log->error($url." could not be retrieved."); - # show throw exception - return undef; + WebGUI::Error::Connection->throw( + error => "Couldn't fetch $url because ".$response->message, + resource => $url, + ); } return $self->set($name, $response->decoded_content, $ttl); } +=head1 EXCEPTIONS + +This class throws a huge number of exceptions about everything you can imagine, and many things you can't. However, because cache should be treated as optional, none of them matter except for testing, debugging, or in very specific use cases. Therefore the best practice is to simply call each method with an eval wrapper, and then not even bother testing for specific exceptions like this: + + my $value = eval { $session->cache->get($key) }; + unless (defined $value) { + $value = $db->fetchValueFromTheDatabase; + } + +=cut + 1; diff --git a/lib/WebGUI/Exception.pm b/lib/WebGUI/Exception.pm index 59525b158..634356477 100644 --- a/lib/WebGUI/Exception.pm +++ b/lib/WebGUI/Exception.pm @@ -51,8 +51,6 @@ These exception classes are defined in this class: =cut -use Exception::Class ( - #------------------------------------------------------------------- =head2 WebGUI::Error @@ -85,11 +83,6 @@ A read only exception method that returns the package name where the exception w =cut - 'WebGUI::Error' => { - description => "A general error occured.", - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::OverrideMe @@ -98,12 +91,6 @@ An interface was not overriden as expected. =cut - 'WebGUI::Error::OverrideMe' => { - isa => 'WebGUI::Error', - description => 'This method should be overridden by subclasses.', - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::MethodNotFound @@ -116,13 +103,6 @@ The method called. =cut - 'WebGUI::Error::MethodNotFound' => { - isa => 'WebGUI::Error', - description => q|Called a method that doesn't exist.|, - fields => 'method' - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::InvalidObject @@ -139,13 +119,6 @@ The object type we got. =cut - 'WebGUI::Error::InvalidObject' => { - isa => 'WebGUI::Error::InvalidParam', - description => "Expected to get a reference to an object type that wasn't gotten.", - fields => ["expected","got"], - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::InvalidParam @@ -158,13 +131,6 @@ Used to return the bad parameter, if present. =cut - 'WebGUI::Error::InvalidParam' => { - isa => 'WebGUI::Error', - description => "Expected to get a param we didn't get.", - fields => ["param"], - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::ObjectNotFound @@ -177,13 +143,6 @@ The id of the object to be retrieved. =cut - 'WebGUI::Error::ObjectNotFound' => { - isa => 'WebGUI::Error', - description => "The object you were trying to retrieve does not exist.", - fields => ["id"], - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::ObjectNotFound::Template @@ -196,13 +155,6 @@ The id of the object to be retrieved. =cut - 'WebGUI::Error::ObjectNotFound::Template' => { - isa => 'WebGUI::Error', - description => "The template an asset was trying to retrieve does not exist.", - fields => [qw/templateId assetId/], - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::InvalidFile @@ -219,13 +171,6 @@ The line the error was found on. =cut - 'WebGUI::Error::InvalidFile' => { - isa => 'WebGUI::Error', - description => "The file you have provided has errors.", - fields => [qw{ brokenFile brokenLine }], - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::Template @@ -234,12 +179,6 @@ Used when a template has parsing errors. ISA WebGUI::Error. =cut - 'WebGUI::Error::Template' => { - isa => 'WebGUI::Error', - description => "A template has errors that prevent it from being processed.", - }, - - #------------------------------------------------------------------- =head2 WebGUI::Error::Connection @@ -252,6 +191,67 @@ The name or configuration or URL of the resource trying to be accessed. =cut +use Exception::Class ( + + 'WebGUI::Error' => { + description => "A general error occured.", + }, + + + 'WebGUI::Error::OverrideMe' => { + isa => 'WebGUI::Error', + description => 'This method should be overridden by subclasses.', + }, + + + 'WebGUI::Error::MethodNotFound' => { + isa => 'WebGUI::Error', + description => q|Called a method that doesn't exist.|, + fields => 'method' + }, + + + 'WebGUI::Error::InvalidObject' => { + isa => 'WebGUI::Error::InvalidParam', + description => "Expected to get a reference to an object type that wasn't gotten.", + fields => ["expected","got"], + }, + + + 'WebGUI::Error::InvalidParam' => { + isa => 'WebGUI::Error', + description => "Expected to get a param we didn't get.", + fields => ["param"], + }, + + + 'WebGUI::Error::ObjectNotFound' => { + isa => 'WebGUI::Error', + description => "The object you were trying to retrieve does not exist.", + fields => ["id"], + }, + + + 'WebGUI::Error::ObjectNotFound::Template' => { + isa => 'WebGUI::Error', + description => "The template an asset was trying to retrieve does not exist.", + fields => [qw/templateId assetId/], + }, + + + 'WebGUI::Error::InvalidFile' => { + isa => 'WebGUI::Error', + description => "The file you have provided has errors.", + fields => [qw{ brokenFile brokenLine }], + }, + + + 'WebGUI::Error::Template' => { + isa => 'WebGUI::Error', + description => "A template has errors that prevent it from being processed.", + }, + + 'WebGUI::Error::Connection' => { isa => 'WebGUI::Error', description => "Couldn't establish a connection.", diff --git a/lib/WebGUI/Session.pm b/lib/WebGUI/Session.pm index 4e0f6ad92..e4a0c139b 100644 --- a/lib/WebGUI/Session.pm +++ b/lib/WebGUI/Session.pm @@ -467,7 +467,6 @@ sub open { $sessionId = $self->id->generate unless $self->id->valid($sessionId); my $noFuss = shift; $self->{_var} = WebGUI::Session::Var->new($self,$sessionId, $noFuss); - $self->errorHandler->warn("You've disabled cache in your config file and that can cause many problems on a production site.") if ($config->get("disableCache")); return $self; } diff --git a/lib/WebGUI/Session/Stow.pm b/lib/WebGUI/Session/Stow.pm index 5345b07cf..181517cad 100644 --- a/lib/WebGUI/Session/Stow.pm +++ b/lib/WebGUI/Session/Stow.pm @@ -118,7 +118,6 @@ sub get { my $self = shift; my $var = shift; my $opt = shift || {}; - return undef if $self->session->config->get("disableCache"); my $value = $self->{_data}{$var}; return undef unless defined $value; my $ref = ref $value; @@ -190,8 +189,6 @@ The value of the stow variable. Any scalar or reference. sub set { my $self = shift; - $self->session->errorHandler->debug('Stow->set() is being called but cache has been disabled') - if $self->session->config->get("disableCache"); my $name = shift; my $value = shift; return undef unless ($name); diff --git a/t/Cache.t b/t/Cache.t index 2cf4ea095..f5684b189 100644 --- a/t/Cache.t +++ b/t/Cache.t @@ -45,9 +45,9 @@ my ($a, $b) = @{$cache->mget(["Shawshank",["andy", "dufresne"]])}; is($a, "Prison", "mget first value"); is($b, "Prisoner", "mget second value"); $cache->delete("Shawshank"); -is($cache->get("Shawshank"), undef, 'delete'); +is(eval{$cache->get("Shawshank")}, undef, 'delete'); $cache->flush; -is($cache->get(["andy", "dufresne"]), undef, 'flush'); +is(eval{$cache->get(["andy", "dufresne"])}, undef, 'flush'); $cache->setByHttp("google", "http://www.google.com/"); cmp_ok($cache->get("google"), 'ne', '', 'setByHttp');