From a4e087d6af60819add9645e5a507e5f65fe282cb Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 10 Jun 2016 09:30:51 +0200 Subject: [PATCH] Bug 16636: [QA Follow-up] Make BakerTaylor plack safe Content-Type: text/plain; charset=utf-8 Initialize file level lexicals each call. Do not call _initialize outside the module. Adjust test by mocking preferences. Test plan: Run t/db_dependent/External_BakerTaylor.t. Signed-off-by: Marcel de Rooy Tested module with trivial script under Plack/memcached by toggling the associated preferences. --- C4/External/BakerTaylor.pm | 21 +++++++++++++------ t/db_dependent/External_BakerTaylor.t | 37 ++++++++++++++++----------------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/C4/External/BakerTaylor.pm b/C4/External/BakerTaylor.pm index 3f52a57..99d4dd5 100644 --- a/C4/External/BakerTaylor.pm +++ b/C4/External/BakerTaylor.pm @@ -1,4 +1,5 @@ package C4::External::BakerTaylor; + # Copyright (C) 2008 LibLime # # @@ -19,17 +20,14 @@ package C4::External::BakerTaylor; use XML::Simple; use LWP::Simple; -# use LWP::UserAgent; use HTTP::Request::Common; + use C4::Context; use C4::Debug; -use strict; -use warnings; +use Modern::Perl; use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $VERSION); -use vars qw($user $pass $agent $image_url $link_url); -&initialize; BEGIN { require Exporter; @@ -39,7 +37,10 @@ BEGIN { %EXPORT_TAGS = (all=>\@EXPORT_OK); } -sub initialize { +# These variables are plack safe: they are initialized each time +my ( $user, $pass, $agent, $image_url, $link_url ); + +sub _initialize { $user = (@_ ? shift : C4::Context->preference('BakerTaylorUsername') ) || ''; # LL17984 $pass = (@_ ? shift : C4::Context->preference('BakerTaylorPassword') ) || ''; # CC82349 $link_url = (@_ ? shift : C4::Context->preference('BakerTaylorBookstoreURL')); @@ -49,24 +50,31 @@ sub initialize { } sub image_url { + _initialize(); ($user and $pass) or return; my $isbn = (@_ ? shift : ''); $isbn =~ s/(p|-)//g; # sanitize return $image_url . $isbn; } + sub link_url { + _initialize(); my $isbn = (@_ ? shift : ''); $isbn =~ s/(p|-)//g; # sanitize $link_url or return; return $link_url . $isbn; } + sub content_cafe_url { + _initialize(); ($user and $pass) or return; my $isbn = (@_ ? shift : ''); $isbn =~ s/(p|-)//g; # sanitize return "http://contentcafe2.btol.com/ContentCafeClient/ContentCafe.aspx?UserID=$user&Password=$pass&Options=Y&ItemKey=$isbn"; } + sub http_jacket_link { + _initialize(); my $isbn = shift or return; $isbn =~ s/(p|-)//g; # sanitize my $image = availability($isbn); @@ -78,6 +86,7 @@ sub http_jacket_link { } sub availability { + _initialize(); my $isbn = shift or return; ($user and $pass) or return; $isbn =~ s/(p|-)//g; # sanitize diff --git a/t/db_dependent/External_BakerTaylor.t b/t/db_dependent/External_BakerTaylor.t index e6e6118..cbaa56e 100755 --- a/t/db_dependent/External_BakerTaylor.t +++ b/t/db_dependent/External_BakerTaylor.t @@ -2,33 +2,32 @@ # some simple tests of the elements of C4::External::BakerTaylor that do not require a valid username and password -use strict; -use warnings; +use Modern::Perl; use Test::More tests => 9; +use t::lib::Mocks; BEGIN { use_ok('C4::External::BakerTaylor'); } -# for testing, to avoid using C4::Context -my $username="testing_username"; -my $password="testing_password"; +# test with mocked prefs +my $username= "testing_username"; +my $password= "testing_password"; +my $link_url = "http://wrongexample.com?ContentCafe.aspx?UserID=$username"; -# taken from C4::External::BakerTaylor::initialize -my $image_url = "http://contentcafe2.btol.com/ContentCafe/Jacket.aspx?UserID=$username&Password=$password&Options=Y&Return=T&Type=S&Value="; - -# test without initializing -is( C4::External::BakerTaylor::image_url(), undef, "testing image url pre initilization"); -is( C4::External::BakerTaylor::link_url(), undef, "testing link url pre initilization"); -is( C4::External::BakerTaylor::content_cafe_url(""), undef, "testing content cafe url pre initilization"); -is( C4::External::BakerTaylor::http_jacket_link(""), undef, "testing http jacket link pre initilization"); -is( C4::External::BakerTaylor::availability(""), undef, "testing availability pre initilization"); +t::lib::Mocks::mock_preference( 'BakerTaylorUsername', $username ); +t::lib::Mocks::mock_preference( 'BakerTaylorPassword', $password ); +t::lib::Mocks::mock_preference( 'BakerTaylorBookstoreURL', $link_url ); -# intitialize -C4::External::BakerTaylor::initialize($username, $password, "link_url"); +my $image_url = "http://contentcafe2.btol.com/ContentCafe/Jacket.aspx?UserID=$username&Password=$password&Options=Y&Return=T&Type=S&Value="; +my $content_cafe = "http://contentcafe2.btol.com/ContentCafeClient/ContentCafe.aspx?UserID=$username&Password=$password&Options=Y&ItemKey="; -# testing basic results +is( C4::External::BakerTaylor::image_url(), $image_url, "testing default image url"); is( C4::External::BakerTaylor::image_url("aa"), $image_url."aa", "testing image url construction"); -is( C4::External::BakerTaylor::link_url("bb"), "link_urlbb", "testing link url construction"); -is( C4::External::BakerTaylor::content_cafe_url("cc"), "http://contentcafe2.btol.com/ContentCafeClient/ContentCafe.aspx?UserID=$username&Password=$password&Options=Y&ItemKey=cc", "testing content cafe url construction"); +is( C4::External::BakerTaylor::link_url(), $link_url, "testing default link url"); +is( C4::External::BakerTaylor::link_url("bb"), "${link_url}bb", "testing link url construction"); +is( C4::External::BakerTaylor::content_cafe_url(""), $content_cafe, "testing default content cafe url"); +is( C4::External::BakerTaylor::content_cafe_url("cc"), "${content_cafe}cc", "testing content cafe url construction"); +is( C4::External::BakerTaylor::http_jacket_link(""), undef, "testing empty http jacket link"); +is( C4::External::BakerTaylor::availability(""), undef, "testing empty availability"); -- 1.7.10.4