From 735b6aa508a448dc4e459e17f3318c12ce87a527 Mon Sep 17 00:00:00 2001 From: Jeff Godin Date: Wed, 25 Apr 2012 04:23:28 -0400 Subject: [PATCH] UPC lookups in Syndetics.pm and improvements to added content by record ID Teach OpenILS::WWW::AddedContent::Syndetic to use the new added content by record ID. Also, teach it to pass UPC when requesting additional content from the source. Use Business::ISBN to clean ISBN values. If we find no valid ISBN and no UPC values in the record, do not attempt to call an AC handler. This will need to change when we have AC handlers that rely on other possible keys. Caching improvement: serve from cache before fetching keys from the bib record in the database. Signed-off-by: Jeff Godin Signed-off-by: Ben Shum --- .../src/perlmods/lib/OpenILS/WWW/AddedContent.pm | 90 ++++++++++++++++------ .../lib/OpenILS/WWW/AddedContent/Syndetic.pm | 24 ++++-- 2 files changed, 82 insertions(+), 32 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm index dcd5d1fa4f..a1e14e6e1b 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm @@ -20,6 +20,8 @@ use OpenILS::Utils::CStoreEditor; use LWP::UserAgent; use MIME::Base64; +use Business::ISBN; + my $AC = __PACKAGE__; @@ -77,62 +79,102 @@ sub child_init { sub handler { my $r = shift; + + # If the URL requested matches a file on the filesystem, have Apache serve that file + # this allows for local content (most typically images) to be used for some requests return Apache2::Const::DECLINED if (-e $r->filename); my $cgi = CGI->new; my @path_parts = split( /\//, $r->unparsed_uri ); - my $type = $path_parts[-3]; - my $format = $path_parts[-2]; - my $id = $path_parts[-1]; + + # Intended URL formats + # /opac/extras/ac/jacket/medium/ISBN_VALUE -- classic keyed-off-isbn + # /opac/extras/ac/-3/-2/-1 + # /opac/extras/ac/jacket/medium/r/RECORD_ID -- provide record id (bre.id) + # /opac/extras/ac/-4/-3/-2/-1 + # /opac/extras/ac/jacket/medium/m/RECORD_ID -- XXX: future use for metarecord id + + my $keytype_in_url = $path_parts[-2]; # if not in one of m, r, this will be the $format + + my $type; + my $format; + my $keytype; + my $keyvalue; + + if ($keytype_in_url =~ m/^(r|m)$/) { + $type = $path_parts[-4]; + $format = $path_parts[-3]; + $keyvalue = $path_parts[-1]; # a record/metarecord id + $keytype = 'record'; + } else { + $type = $path_parts[-3]; + $format = $path_parts[-2]; + $keyvalue = $path_parts[-1]; # an isbn + $keytype = 'isbn'; + } + my $res; + my $keyhash; + my $cachekey; + + $cachekey = ($keytype eq "isbn") ? $keyvalue : $keytype . '_' . $keyvalue; child_init() unless $handler; - return Apache2::Const::NOT_FOUND unless $handler and $type and $format and $id; + return Apache2::Const::NOT_FOUND unless $handler and $type and $format and $cachekey; my $err; my $data; my $method = "${type}_${format}"; return Apache2::Const::NOT_FOUND unless $handler->can($method); - return $res if defined($res = $AC->serve_from_cache($type, $format, $id)); + return $res if defined($res = $AC->serve_from_cache($type, $format, $cachekey)); return Apache2::Const::NOT_FOUND unless $AC->lookups_enabled; - my $key_data = get_rec_keys($id); - my @isbns = grep {$_->{tag} eq '020'} @$key_data; - my @upcs = grep {$_->{tag} eq '024'} @$key_data; - - my $keys = { - isbn => [map {$_->{value}} @isbns], - upc => [map {$_->{value}} @upcs] - }; - - # TODO clean isbn + if ($keytype eq "isbn") { # if this request uses isbn for the key + # craft a hash with the single isbn, because that's all we will have + $keyhash = {}; + $keyhash->{"isbn"} = [$keyvalue]; + } else { + my $key_data = get_rec_keys($keyvalue); + my @isbns = grep {$_->{tag} eq '020'} @$key_data; + my @upcs = grep {$_->{tag} eq '024'} @$key_data; + + map { + my $isbn_obj = Business::ISBN->new($_->{value}); + my $isbn_str; + $isbn_str = $isbn_obj->as_string([]) if defined($isbn_obj); + $_->{value} = $isbn_str; + undef $_ if !defined($_->{value}); + } @isbns; + + $keyhash = { + isbn => [map {$_->{value}} @isbns], + upc => [map {$_->{value}} @upcs] + }; + } - # XXX DEBUG - use OpenSRF::Utils::JSON; - $logger->info("Added Content Keys: " . OpenSRF::Utils::JSON->perl2JSON($keys)); + return Apache2::Const::NOT_FOUND unless @{$keyhash->{isbn}} || @{$keyhash->{upc}}; try { - $data = $handler->$method($keys); + $data = $handler->$method($keyhash); } catch Error with { $err = shift; decr_error_countdown(); - $logger->debug("added content handler failed: $method($id) => $err"); + $logger->debug("added content handler failed: $method($keytype/$keyvalue) => $err"); # XXX: logs unhelpful hashref }; return Apache2::Const::NOT_FOUND if $err; - next unless $data; if(!$data) { # if the AC lookup found no corresponding data, cache that information - $logger->debug("added content handler returned no results $method($id)") unless $data; - $AC->cache_result($type, $format, $id, {nocontent=>1}); + $logger->debug("added content handler returned no results $method($keytype/$keyvalue)") unless $data; + $AC->cache_result($type, $format, $cachekey, {nocontent=>1}); return Apache2::Const::NOT_FOUND; } $AC->print_content($data); - $AC->cache_result($type, $format, $id, $data); + $AC->cache_result($type, $format, $cachekey, $data); reset_error_countdown(); return Apache2::Const::OK; diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent/Syndetic.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent/Syndetic.pm index 9eff0ae21b..e1b992c0ce 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent/Syndetic.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent/Syndetic.pm @@ -30,21 +30,21 @@ sub userid { # -------------------------------------------------------------------------- sub jacket_small { - my( $self, $key ) = @_; + my( $self, $keys ) = @_; return $self->send_img( - $self->fetch_response('sc.gif', $key, 1)); + $self->fetch_response('sc.gif', $keys, 1)); } sub jacket_medium { - my( $self, $key ) = @_; + my( $self, $keys ) = @_; return $self->send_img( - $self->fetch_response('mc.gif', $key, 1)); + $self->fetch_response('mc.gif', $keys, 1)); } sub jacket_large { - my( $self, $key ) = @_; + my( $self, $keys ) = @_; return $self->send_img( - $self->fetch_response('lc.gif', $key, 1)); + $self->fetch_response('lc.gif', $keys, 1)); } # -------------------------------------------------------------------------- @@ -280,9 +280,17 @@ sub fetch_content { # returns the HTTP response object from the URL fetch sub fetch_response { - my( $self, $page, $key, $notype ) = @_; + my( $self, $page, $keys, $notype ) = @_; my $uname = $self->userid; - my $url = $self->base_url . "?isbn=$key/$page&client=$uname" . (($notype) ? '' : "&type=rw12"); + + # Fetch single isbn and single upc + my $isbn = $keys->{isbn}[0]; + my $upc = $keys->{upc}[0]; + + $isbn = '' if !defined($isbn); + $upc = '' if !defined($upc); + + my $url = $self->base_url . "?isbn=$isbn/$page&upc=$upc&client=$uname" . (($notype) ? '' : "&type=rw12"); return $AC->get_url($url); } -- 2.11.0