From a12aceb678007b7d17a4a404938a65a72754c149 Mon Sep 17 00:00:00 2001
From: Jeff Godin <jgodin@tadl.org>
Date: Wed, 2 May 2012 10:29:36 -0400
Subject: [PATCH] Fix broken ISBN cleaning, handle missing keys

Previous attempt at ISBN cleaning was broken. This approach
is not broken.

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.

Also:
* Fixed a stray "next" left over from an earlier re-factor.
* Removed some debugging.

Signed-off-by: Jeff Godin <jgodin@tadl.org>
---
 .../src/perlmods/lib/OpenILS/WWW/AddedContent.pm   | 49 +++++++++++-----------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/AddedContent.pm
index 90a9de2418..169a1830b2 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__;
 
 
@@ -114,44 +116,44 @@ sub handler {
     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 $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, $cachekey));
+    return Apache2::Const::NOT_FOUND unless $AC->lookups_enabled;
+
     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];
-        $cachekey = $keyvalue;
     } else {
         my $key_data = get_rec_keys($keyvalue);
-        my @raw_isbns = grep {$_->{tag} eq '020'} @$key_data;
+        my @isbns = grep {$_->{tag} eq '020'} @$key_data;
         my @upcs = grep {$_->{tag} eq '024'} @$key_data;
 
-        my @isbns = [map {
-            my $isbn_obj = Business::ISBN->new($_);
-            return $isbn_obj->as_string([]);
-        } @raw_isbns];
+        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]
         };
-
-        $cachekey = $keytype . '_' . $keyvalue;
     }
 
-    child_init() unless $handler;
-
-    return Apache2::Const::NOT_FOUND unless $handler and $type and $format and $keyhash;
-
-    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, $cachekey));
-    return Apache2::Const::NOT_FOUND unless $AC->lookups_enabled;
-
-    # XXX DEBUG
-    use OpenSRF::Utils::JSON;
-    $logger->info("Added Content Keys: " . OpenSRF::Utils::JSON->perl2JSON($keyhash));
+    return Apache2::Const::NOT_FOUND unless @{$keyhash->{isbn}} || @{$keyhash->{upc}};
 
     try {
         $data = $handler->$method($keyhash);
@@ -162,7 +164,6 @@ sub handler {
     };
 
     return Apache2::Const::NOT_FOUND if $err;
-    next unless $data;
 
     if(!$data) {
         # if the AC lookup found no corresponding data, cache that information
-- 
2.11.0