From 8de4e0b29bc6a0ffbd00555a7c6b07cff1684146 Mon Sep 17 00:00:00 2001 From: Art Rhyno Date: Wed, 14 Dec 2011 20:13:30 -0500 Subject: [PATCH] Timeout for resolver interactions These changes add some timeout options for using the resolver setup. For example: request open-ils.resolver open-ils.resolver.resolve_holdings "issn", "0013-0618", "http://sfx.scholarsportal.info/windsor", 10 where "10" is the number of seconds for a timeout. A default timeout can be specified in the opensrf.xml file in the ResolverResolver section as well: 30 from TPAC, a request can be also include a timeout option: [% IF openurl.enabled == 'true'; FOR issn IN args.resolver_issns; resolver = ResolverResolver.resolve_issn(issn, openurl.baseurl,20); FOR res IN resolver; %] where "20" is the number of seconds for a timeout. In working through this, I found some bugs in my resolver collection code in misc_util.tt2 for isbns which I have addressed. --- .../lib/OpenILS/Application/ResolverResolver.pm | 190 ++++++++++++++++++++- .../lib/Template/Plugin/ResolverResolver.pm | 10 +- Open-ILS/src/templates/opac/parts/misc_util.tt2 | 45 ++--- 3 files changed, 215 insertions(+), 30 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/ResolverResolver.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/ResolverResolver.pm index 76a84edff7..2a84a181be 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/ResolverResolver.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/ResolverResolver.pm @@ -83,6 +83,8 @@ my $prefix = "open-ils.resolver_"; # Prefix for caching values my $cache; my $cache_timeout; my $default_url_base; # Default resolver location +my $resolver_type; # Default resolver type +my $default_lwp_timeout; # Default browser timeout our ($ua, $parser); @@ -94,9 +96,15 @@ sub initialize { "apps", "open-ils.resolver", "app_settings", "cache_timeout" ) || 300; $default_url_base = $sclient->config_value( "apps", "open-ils.resolver", "app_settings", "default_url_base"); + $resolver_type = $sclient->config_value( + "apps", "open-ils.resolver", "app_settings", "resolver_type"); + # We set a browser timeout + $default_lwp_timeout = $sclient->config_value( + "apps", "open-ils.resolver", "app_settings", "lwp_timeout" ) || 60; } sub child_init { + # We need a User Agent to speak to the SFX beast $ua = new LWP::UserAgent; $ua->agent('SameOrigin/1.0'); @@ -111,6 +119,140 @@ sub resolve_holdings { my $id_type = shift; # keep it simple for now, either 'issn' or 'isbn' my $id_value = shift; # the normalized ISSN or ISBN my $url_base = shift || $default_url_base; + my $lwp_timeout = shift || $default_lwp_timeout; + + # Need some sort of timeout in case resolver is unreachable + $ua->timeout($lwp_timeout); + + if ($resolver_type eq 'cufts') { + return cufts_holdings($self,$conn,$id_type,$id_value,$url_base); + } else { + return sfx_holdings($self,$conn,$id_type,$id_value,$url_base); + } +} + +sub cufts_holdings{ + + my $self = $_[0]; + my $conn = $_[1]; + my $id_type = $_[2]; + my $id_value = $_[3]; + my $url_base = $_[4]; + + # We'll use this in our cache key + my $method = $self->api_name; + + # We might want to return raw JSON for speedier responses + my $format = 'fieldmapper'; + if ($self->api_name =~ /raw$/) { + $format = 'raw'; + } + + # Nice little CUFTS OpenURL request + my $url_args = '?'; + + if ($id_type eq 'issn') { + $url_args .= "&issn=$id_value"; + } elsif ($id_type eq 'isbn') { + $url_args .= "&isbn=$id_value"; + } + + my $ckey = $prefix . $method . $url_base . $id_type . $id_value; + + # Check the cache to see if we've already looked this up + # If we have, shortcut our return value + my $result = $cache->get_cache($ckey) || undef; + if ($result) { + $logger->info("Resolver found a cache hit"); + return $result; + } + + my $res = undef; + + # Let's see what we we're trying to request + $logger->info("Resolving the following request: $url_base$url_args"); + + # We attempt to deal with potential problems in request + eval { + $res = $ua->get("$url_base$url_args"); + } or do { + $logger->info("execution error"); + return bow_out_gracefully($url_base . '?ctx_ver=Z39.88-2004&rft.issn=' . $id_value, + 'Check link for additional holdings information.'); + }; + + if ($res->status_line =~ /timeout/) { + $logger->info("timeout error"); + return bow_out_gracefully($url_base . '?ctx_ver=Z39.88-2004&rft.issn=' . $id_value, + 'Check link for additional holdings information.'); + } + + my $xml = $res->content; + my $parsed_cufts = $parser->parse_string($xml); + + my (@targets) = $parsed_cufts->findnodes('/CUFTS/resource/service[@name="journal"]'); + + my @cufts_result; + foreach my $target (@targets) { + my %full_txt; + + # Ensure we have a name and especially URL to return + $full_txt{'name'} = $target->findvalue('../@name[1]'); + $full_txt{'url'} = $target->findvalue('./result/url') || next; + $full_txt{'coverage'} = $target->findvalue('./result/ft_start_date') . ' - ' . $target->findvalue('./result/ft_end_date'); + my $embargo = ""; + my $days_embargo = $target->findvalue('./result/embargo_days') || ''; + if (length($days_embargo) > 0) { + $days_embargo = $days_embargo . " days "; + } + my $months_embargo = $target->findvalue('./result/embargo_months') || ''; + if (length($months_embargo) > 0) { + $months_embargo = $months_embargo . " months "; + } + my $years_embargo = $target->findvalue('./result/embargo_years') || ''; + if (length($years_embargo) > 0) { + $years_embargo = $years_embargo . " years "; + } + if (length($years_embargo . $months_embargo . $days_embargo) > 0) { + $embargo = "(most recent " . $years_embargo . $months_embargo . $days_embargo . "unavailable due to publisher restrictions)"; + } + $full_txt{'embargo'} = $embargo; + + if ($format eq 'raw') { + push @cufts_result, { + public_name => $full_txt{'name'}, + target_url => $full_txt{'url'}, + target_coverage => $full_txt{'coverage'}, + target_embargo => $full_txt{'embargo'}, + }; + } else { + my $rhr = Fieldmapper::resolver::holdings_record->new; + $rhr->public_name($full_txt{'name'}); + $rhr->target_url($full_txt{'url'}); + $rhr->target_coverage($full_txt{'coverage'}); + $rhr->target_embargo($full_txt{'embargo'}); + push @cufts_result, $rhr; + } + } + + # Stuff this into the cache + $cache->put_cache($ckey, \@cufts_result, $cache_timeout); + + # Don't return the list unless it contains results + if (scalar(@cufts_result)) { + return \@cufts_result; + } + + return undef; +} + +sub sfx_holdings{ + + my $self = $_[0]; + my $conn = $_[1]; + my $id_type = $_[2]; + my $id_value = $_[3]; + my $url_base = $_[4]; # We'll use this in our cache key my $method = $self->api_name; @@ -143,14 +285,27 @@ sub resolve_holdings { return $result; } - # Otherwise, let's go and grab the info from the SFX server - my $req = HTTP::Request->new('GET', "$url_base$url_args"); + my $res = undef; # Let's see what we we're trying to request $logger->info("Resolving the following request: $url_base$url_args"); - my $res = $ua->request($req); + # We attempt to deal with potential problems in request + eval { + $res = $ua->get("$url_base$url_args"); + } or do { + $logger->info("execution error"); + return bow_out_gracefully($url_base . '?ctx_ver=Z39.88-2004&rft.issn=' . $id_value, + 'Check link for additional holdings information.'); + }; + + if ($res->status_line =~ /timeout/) { + $logger->info("timeout error"); + return bow_out_gracefully($url_base . '?ctx_ver=Z39.88-2004&rft.issn=' . $id_value, + 'Check link for additional holdings information.'); + } + # All clear my $xml = $res->content; my $parsed_sfx = $parser->parse_string($xml); @@ -194,6 +349,23 @@ sub resolve_holdings { return undef; } +# This uses the resolver structure for passing back a link directly to the resolver +sub bow_out_gracefully { + my $alt_url = $_[0]; + my $reason = $_[1]; + + my @sfx_result; + + push @sfx_result, { + public_name => "Online holdings", + target_url => $alt_url, + target_coverage => $reason, + target_embargo => "", + }; + + return \@sfx_result; +} + __PACKAGE__->register_method( method => 'resolve_holdings', api_name => 'open-ils.resolver.resolve_holdings', @@ -215,6 +387,10 @@ Returns a list of "rhr" objects representing the full-text holdings for a given name => 'url_base', desc => 'The base URL for the resolver and instance', type => 'string' + }, { + name => 'lwp_timeout', + desc => 'The timeout for the LWP request', + type => 'string' }, ], 'return' => { @@ -245,6 +421,10 @@ Returns a list of raw JSON objects representing the full-text holdings for a giv name => 'url_base', desc => 'The base URL for the resolver and instance', type => 'string' + }, { + name => 'lwp_timeout', + desc => 'The timeout for the LWP request', + type => 'string' }, ], 'return' => { @@ -299,6 +479,10 @@ Deletes the cached value of the full-text holdings for a given ISBN or ISSN name => 'url_base', desc => 'The base URL for the resolver and instance', type => 'string' + }, { + name => 'lwp_timeout', + desc => 'The timeout for the LWP request', + type => 'string' }, ], 'return' => { diff --git a/Open-ILS/src/perlmods/lib/Template/Plugin/ResolverResolver.pm b/Open-ILS/src/perlmods/lib/Template/Plugin/ResolverResolver.pm index 88109c2bd2..b81b77f30b 100644 --- a/Open-ILS/src/perlmods/lib/Template/Plugin/ResolverResolver.pm +++ b/Open-ILS/src/perlmods/lib/Template/Plugin/ResolverResolver.pm @@ -62,12 +62,12 @@ sub ResolverResolver::params { sub resolve_issn { - my ($class, $c, $baseurl) = @_; + my ($class, $issn, $baseurl, $timeout) = @_; - if (length($c) <= 9) { + if (length($issn) <= 9) { my $session = OpenSRF::AppSession->create("open-ils.resolver"); - my $request = $session->request("open-ils.resolver.resolve_holdings.raw", "issn", $c, $baseurl)->gather(); + my $request = $session->request("open-ils.resolver.resolve_holdings.raw", "issn", $issn, $baseurl, $timeout)->gather(); if ($request) { return $request; } @@ -79,11 +79,11 @@ sub resolve_issn sub resolve_isbn { - my ($class, $c, $baseurl) = @_; + my ($class, $isbn, $baseurl, $timeout) = @_; my $session = OpenSRF::AppSession->create("open-ils.resolver"); - my $request = $session->request("open-ils.resolver.resolve_holdings.raw", "isbn", $c, $baseurl)->gather(); + my $request = $session->request("open-ils.resolver.resolve_holdings.raw", "isbn", $isbn, $baseurl, $timeout)->gather(); if ($request) { return $request; diff --git a/Open-ILS/src/templates/opac/parts/misc_util.tt2 b/Open-ILS/src/templates/opac/parts/misc_util.tt2 index db30d6b3d7..8e8405e096 100644 --- a/Open-ILS/src/templates/opac/parts/misc_util.tt2 +++ b/Open-ILS/src/templates/opac/parts/misc_util.tt2 @@ -60,36 +60,37 @@ args.holdings = []; args.uris = []; args.issns = []; + args.resolver_isbns = []; + args.resolver_issns = []; # we use $9 of ISBN and ISSN as a flag for e-version - sfx_isbn = xml.findnodes('//*[@tag="020"]/*[@code="9"]'); - IF sfx_isbn; - IF sfx_isbn.textContent == "SFX"; - my_parent = sfx_isbn.parentNode(); - sfx_isbn = my_parent.findnodes('./*[@code="a"]').textContent; - sfx_isbn = sfx_isbn.replace('-', ''); - args.resolver_isbn = sfx_isbn.replace('\ .*', ''); + FOR resolver_isbn IN xml.findnodes('//*[@tag="020"]/*[@code="9"]'); + IF resolver_isbn.textContent == "SFX" || resolver_isbn.textContent == "CUFTS"; + my_parent = resolver_isbn.parentNode(); + FOR resolver_isbn_val IN my_parent.findnodes('./*[@code="a"]'); + args.resolver_isbns.push( + resolver_isbn_val.textContent.replace('-', '').replace('\ .*', '') + ); + END; END; END; - sfx_issn = xml.findnodes('//*[@tag="022"]/*[@code="9"]'); - IF sfx_issn; - IF sfx_issn.textContent == "SFX"; - my_parent = sfx_issn.parentNode(); - sfx_issn = my_parent.findnodes('./*[@code="a"]'); - args.issns.push( - sfx_issn.textContent.replace('[^\d\-X]', '') - ); + FOR resolver_issn IN xml.findnodes('//*[@tag="022"]/*[@code="9"]'); + IF resolver_issn.textContent == "SFX" || resolver_issn.textContent == "CUFTS"; + my_parent = resolver_issn.parentNode(); + FOR resolver_issn_val IN my_parent.findnodes('./*[@code="a"]'); + args.resolver_issns.push( + resolver_issn_val.textContent.replace('[^\d\-X]', '') + ); + END; END; END; - # we snag all issns if no SFX available - IF args.issns.size == 0; - FOR rawissn IN xml.findnodes('//*[@tag="022"]/*[@code="a"]'); - args.issns.push( - rawissn.textContent.replace('[^\d\-X]', '') - ); - END; + # now snag all issns + FOR rawissn IN xml.findnodes('//*[@tag="022"]/*[@code="a"]'); + args.issns.push( + rawissn.textContent.replace('[^\d\-X]', '') + ); END; FOR volume IN xml.findnodes('//*[local-name()="volumes"]/*[local-name()="volume"]'); -- 2.11.0