From 0b2e517259eaf7838bb814d3a36137424df89583 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 9 Oct 2017 15:36:41 -0300 Subject: [PATCH] Bug 19440: Identify overlimit problems in XISBN tests This patch makes C4::XISBN::get_xisbns() return an errors hashref including information of the failing fetches from xisbn services. It tackles the situation of XISBN, which in some cases returns 'overlimit' errors. The patch makes the relevant functions check if the response->{stat} string is 'ok' and returns the string in $errors otherwise. This only happens when in list context. This allows to fix the randomly failing tests while keeping the current behaviour. All this code should be rewritten. It does the job bug doesn't have problems handling or reoprting. This is just a band aid. To test: - Make sure k$ prove t/db_dependent/XISBN.t => SUCCESS :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart (cherry picked from commit e90874241808354777cc1c27ad3ad106ffdc7cd4) Signed-off-by: Fridolin Somers (cherry picked from commit aaf0d3dc474def89ffbb69548ebaf830109143b0) Signed-off-by: Katrin Fischer --- C4/XISBN.pm | 11 +++++++++-- t/db_dependent/XISBN.t | 8 ++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/C4/XISBN.pm b/C4/XISBN.pm index 55126334b3..00a4ce412b 100644 --- a/C4/XISBN.pm +++ b/C4/XISBN.pm @@ -88,7 +88,7 @@ sub _get_biblio_from_xisbn { sub get_xisbns { my ( $isbn ) = @_; - my ($response,$thing_response,$xisbn_response,$syndetics_response); + my ($response,$thing_response,$xisbn_response,$syndetics_response,$errors); # THINGISBN if ( C4::Context->preference('ThingISBN') ) { my $url = "http://www.librarything.com/api/thingISBN/".$isbn; @@ -114,6 +114,8 @@ sub get_xisbns { unless ($reached_limit) { $xisbn_response = _get_url($url,'xisbn'); } + $errors->{xisbn} = $xisbn_response->{ stat } + if $xisbn_response->{ stat } ne 'ok'; } $response->{isbn} = [ @{ $xisbn_response->{isbn} or [] }, @{ $syndetics_response->{isbn} or [] }, @{ $thing_response->{isbn} or [] } ]; @@ -129,7 +131,12 @@ sub get_xisbns { my $xbiblio= _get_biblio_from_xisbn($response_data->{content}); push @xisbns, $xbiblio if $xbiblio; } - return \@xisbns; + if ( wantarray ) { + return (\@xisbns, $errors); + } + else { + return \@xisbns; + } } sub _get_url { diff --git a/t/db_dependent/XISBN.t b/t/db_dependent/XISBN.t index 2f6d63b5b8..df598ceabb 100755 --- a/t/db_dependent/XISBN.t +++ b/t/db_dependent/XISBN.t @@ -25,7 +25,7 @@ $dbh->{AutoCommit} = 0; my $search_module = new Test::MockModule('C4::Search'); $search_module->mock('SimpleSearch', \&Mock_SimpleSearch ); - +my $errors; my $context = C4::Context->new; my ( $biblionumber_tag, $biblionumber_subfield ) = @@ -73,10 +73,10 @@ t::lib::Mocks::mock_preference( 'ThingISBN', 0 ); t::lib::Mocks::mock_preference( 'XISBN', 1 ); my $results_xisbn; -eval { $results_xisbn = C4::XISBN::get_xisbns($isbn1); }; +eval { ($results_xisbn, $errors) = C4::XISBN::get_xisbns($isbn1); }; SKIP: { - skip "Problem retrieving XISBN", 1 - unless $@ eq ''; + skip "Problem retrieving XISBN (" . $errors->{xisbn} . ")", 1 + if $errors->{xisbn}; is( $results_xisbn->[0]->{biblionumber}, $biblionumber3, "Gets correct biblionumber from a book with a similar isbn using XISBN." ); -- 2.39.5