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 <jgodin@tadl.org>
Signed-off-by: Ben Shum <bshum@biblio.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm
index dcd5d1f..a1e14e6 100644 (file)
--- 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 9eff0ae..e1b992c 100644 (file)
--- 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);
 }