Bug 16636: [QA Follow-up] Make BakerTaylor plack safe

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 <m.de.rooy@rijksmuseum.nl>
Tested module with trivial script under Plack/memcached by toggling
the associated preferences.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/External/BakerTaylor.pm b/C4/External/BakerTaylor.pm
index 3f52a57..99d4dd5 100644 (file)
--- a/C4/External/BakerTaylor.pm
+++ b/C4/External/BakerTaylor.pm
@@ -1,4 +1,5 @@
 package C4::External::BakerTaylor;
+
 # Copyright (C) 2008 LibLime
 # <jmf at liblime dot com>
 #
@@ -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 (executable)
--- 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");