From dd704f83890642462fc97f763d56abafc3ff1b47 Mon Sep 17 00:00:00 2001
From: efalcy <efalcy@afi-sa.fr>
Date: Wed, 20 Nov 2019 18:43:36 +0100
Subject: [PATCH] hotline#100763 : catch error when webservice fails on getting
 late loans

---
 VERSIONS_HOTLINE/100763                       |   1 +
 library/Class/User/Cards.php                  |  19 +++
 library/Class/Users.php                       |  25 +---
 .../Class/WebService/SIGB/Opsys/Service.php   | 131 ++++++++++--------
 .../Controller/Action/Helper/Notify.php       |   4 +-
 .../opac/controllers/AuthControllerTest.php   |  58 ++++++++
 .../WebService/SIGB/OpsysServiceTest.php      |  36 ++++-
 7 files changed, 186 insertions(+), 88 deletions(-)
 create mode 100644 VERSIONS_HOTLINE/100763

diff --git a/VERSIONS_HOTLINE/100763 b/VERSIONS_HOTLINE/100763
new file mode 100644
index 00000000000..8fcb3fcd37b
--- /dev/null
+++ b/VERSIONS_HOTLINE/100763
@@ -0,0 +1 @@
+ - ticket #100763 : Compte lecteur : Détection et affichage des erreurs pouvant survenir lors de la récupération des prêts en retard auprès du SIGB
\ No newline at end of file
diff --git a/library/Class/User/Cards.php b/library/Class/User/Cards.php
index 9e73bf671e4..6f4504e78bd 100644
--- a/library/Class/User/Cards.php
+++ b/library/Class/User/Cards.php
@@ -180,6 +180,25 @@ class Class_User_Cards extends Storm_Model_Collection {
                       (new Class_User_ILSSubscription($card))->registerNotificationsOn($notifiable);
                   });
 
+    try {
+      if (!$this->hasPagedLoans() && ($late_loans_count = $this->getLateLoansCount()))
+        $notifiable->notify($this->_plural($late_loans_count,
+                                           '',
+                                           'Vous avez %d document en retard.',
+                                           'Vous avez %d documents en retard.',
+                                           $late_loans_count));
+
+      if ($pullable_items_count = $this->countWaitingToBePulled())
+        $notifiable->notify($this->_plural($pullable_items_count,
+                                           '',
+                                           'Vous avez %d document en attente de retrait.',
+                                           'Vous avez %d documents en attente de retrait.',
+                                           $pullable_items_count));
+    } catch(Exception $e) {
+      $notifiable->notify($this->_('Une erreur est survenue nous empêchant de lister vos prêts.'));
+      $notifiable->notify($this->_('Erreur : "%s"', $e->getMessage()));
+    }
+
     return $this;
   }
 }
\ No newline at end of file
diff --git a/library/Class/Users.php b/library/Class/Users.php
index 876feb1fde1..51851c87134 100644
--- a/library/Class/Users.php
+++ b/library/Class/Users.php
@@ -1725,31 +1725,8 @@ class Class_Users extends Storm_Model_Abstract {
 
 
   public function registerNotificationsOn($notifiable) {
-    $cards = (new Class_User_Cards($this))->registerNotificationsOn($notifiable);
     (new Class_User_CardsNotification($this))->registerNotificationsOn($notifiable);
-
-    $this
-      ->_notifyOn($notifiable,
-                  !$cards->hasPagedLoans() && ($late_loans_count = $cards->getLateLoansCount()),
-                  $this->_plural($late_loans_count,
-                                 '',
-                                 'Vous avez %d document en retard.',
-                                 'Vous avez %d documents en retard.',
-                                 $late_loans_count))
-      ->_notifyOn($notifiable,
-                  $pullable_items_count = $cards->countWaitingToBePulled(),
-                  $this->_plural($pullable_items_count,
-                                 '',
-                                 'Vous avez %d document en attente de retrait.',
-                                 'Vous avez %d documents en attente de retrait.',
-                                 $pullable_items_count));
-  }
-
-
-  protected function _notifyOn($notifiable, $condition, $message) {
-    if ($condition)
-      $notifiable->notify($message);
-    return $this;
+    (new Class_User_Cards($this))->registerNotificationsOn($notifiable);
   }
 
 
diff --git a/library/Class/WebService/SIGB/Opsys/Service.php b/library/Class/WebService/SIGB/Opsys/Service.php
index 862ac158ddc..6c044281cf9 100644
--- a/library/Class/WebService/SIGB/Opsys/Service.php
+++ b/library/Class/WebService/SIGB/Opsys/Service.php
@@ -154,18 +154,19 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
   }
 
 
-  public function connect(){
-    try {
-      $osr = $this->search_client->OuvrirSession(new OuvrirSession());
-      $this->guid = $osr->getGUID();
-    } catch (SoapFault $e) {
-      return false;
-    }
-    return ($this->isConnected());
+  public function connect() {
+    return $this
+      ->_withClientTryOrDo(function($client)
+                           {
+                             $osr = $client->OuvrirSession(new OuvrirSession());
+                             $this->guid = $osr->getGUID();
+                             return $this->isConnected();
+                           },
+                           function($exception) { return false; });
   }
 
 
-    /** @codeCoverageIgnore */
+  /** @codeCoverageIgnore */
   protected function _dumpSoapTrace() {
     var_dump($this->search_client->__getLastRequestHeaders());
     var_dump($this->search_client->__getLastRequest());
@@ -175,26 +176,23 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
 
 
 
-  public function disconnect(){
-    $fs=new FermerSession($this->guid);
-    try {
-      $fsr = $this->search_client->FermerSession($fs);
-    } catch (Exception $e) {
-        //Aloes V190 plante parfois sur les FermerSession
-    }
+  public function disconnect() {
+    //Aloes V190 plante parfois sur les FermerSession
+    $this
+      ->_withClientTryOrDo(function($client)
+                           {
+                             $client->FermerSession(new FermerSession($this->guid));
+                           });
   }
 
 
+  /**
+   * @param $emprunteur Class_WebService_SIGB_Emprunteur
+   * @return array
+   */
   public function getEmpruntsOf($emprunteur) {
-    // prets pas en retard
-    $liste_prets_result = $this->search_client->EmprListerEntite(
-                                          EmprListerEntite::prets($this->guid));
-    $prets = $liste_prets_result->getEntites('Class_WebService_SIGB_Emprunt');
-
-    // prets en retard
-    $liste_prets_retard = $this->search_client->EmprListerEntite(
-                                  EmprListerEntite::prets_en_retard($this->guid));
-    $prets_retard = $liste_prets_retard->getEntites('Class_WebService_SIGB_Emprunt');
+    $prets = $this->_getEmpruntsOfType(EmprListerEntite::prets($this->guid));
+    $prets_retard = $this->_getEmpruntsOfType(EmprListerEntite::prets_en_retard($this->guid));
     foreach($prets_retard as $retard)
       $retard->setEnRetard(true);
 
@@ -202,10 +200,27 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
   }
 
 
+  protected function _getEmpruntsOfType($type) {
+    return $this->search_client->EmprListerEntite($type)
+                               ->getEntites('Class_WebService_SIGB_Emprunt');
+  }
+
+
+  protected function _withClientTryOrDo($action, $on_error=null) {
+    try {
+      return $action($this->search_client);
+    } catch (Exception $e) {
+      return null === $on_error
+        ? null
+        : $on_error($e);
+    }
+  }
+
+
   public function getReservationsOf($emprunteur) {
-    $reserv_result = $this->search_client->EmprListerEntite(
-                                  EmprListerEntite::reservations($this->guid));
-    return $reserv_result->getEntites('Class_WebService_SIGB_Opsys_Reservation');
+    return $this->search_client
+      ->EmprListerEntite(EmprListerEntite::reservations($this->guid))
+      ->getEntites('Class_WebService_SIGB_Opsys_Reservation');
   }
 
 
@@ -231,13 +246,18 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
    * @return Class_WebService_SIGB_Emprunteur
    */
   public function authentifierEmprunteur($user) {
-    $auth = new EmprAuthentifier($this->guid, $user->getLogin(), $user->getPassword());
+    $auth_result = $this
+      ->_withClientTryOrDo(function($client) use($user)
+                           {
+                             $auth = new EmprAuthentifier($this->guid,
+                                                          $user->getLogin(),
+                                                          $user->getPassword());
 
-    try {
-      $auth_result = $this->search_client->EmprAuthentifier($auth);
-    } catch (Exception $e) {
+                             return $client->EmprAuthentifier($auth);
+                           });
+
+    if (!$auth_result)
       return Class_WebService_SIGB_Emprunteur::nullInstance();
-    }
 
     $emprunteur = $auth_result
       ->createEmprunteur()
@@ -292,12 +312,12 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
   public function reserverExemplaire($user, $exemplaire, $code_annexe){
     $exemplaire_sigb = $this->getExemplaire($exemplaire->getIdOrigine(), $exemplaire->getCodeBarres());
     $emprunteur = $this->authentifierEmprunteur($user);
-    $reserv_result = $this->search_client
-      ->EmprReserver(
-                     new EmprReserver($this->guid,
+
+    return $this->search_client
+      ->EmprReserver(new EmprReserver($this->guid,
                                       $exemplaire_sigb->getId(),
-                                      $this->_is_reserver_retrait_bib_abonne ? '' : $code_annexe));
-    return $reserv_result->getReussite();
+                                      $this->_is_reserver_retrait_bib_abonne ? '' : $code_annexe))
+      ->getReussite();
   }
 
 
@@ -308,10 +328,10 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
    */
   public function supprimerReservation($user, $reservation_id){
     $emprunteur = $this->authentifierEmprunteur($user);
-    $supp_result = $this->search_client->EmprSupprResa(
-                                                new EmprSupprResa($this->guid,
-                                                                  $reservation_id));
-    return $supp_result->getReussite();
+
+    return $this->search_client
+      ->EmprSupprResa(new EmprSupprResa($this->guid, $reservation_id))
+      ->getReussite();
   }
 
 
@@ -322,21 +342,21 @@ class Class_WebService_SIGB_Opsys_Service extends Class_WebService_SIGB_Abstract
    */
   public function prolongerPret($user, $pret_id){
     $emprunteur = $this->authentifierEmprunteur($user);
-    $prolong_result = $this->search_client->EmprProlong(
-                                                 new EmprProlong($this->guid,
-                                                                 $pret_id));
-    return $prolong_result->getReussite();
+
+    return $this->search_client
+      ->EmprProlong(new EmprProlong($this->guid, $pret_id))
+      ->getReussite();
   }
 
 
-  public function getNotice($id){
-    try {
-      $notice_result = $this->search_client->RecupererNotice(
-        new RecupererNotice($this->guid, $id));
-      return $notice_result->createNotice();
-    } catch (Exception $e) {
-      //$this->_dumpSoapTrace();
-    }
+  public function getNotice($id) {
+    return $this
+      ->_withClientTryOrDo(function($client) use($id)
+                           {
+                             return $client
+                               ->RecupererNotice(new RecupererNotice($this->guid, $id))
+                               ->createNotice();
+                           });
   }
 
 
@@ -1861,6 +1881,3 @@ class ImportSousChamp {
   public $Etiquette; // string
   public $Description; // string
 }
-
-
-?>
\ No newline at end of file
diff --git a/library/ZendAfi/Controller/Action/Helper/Notify.php b/library/ZendAfi/Controller/Action/Helper/Notify.php
index 72286b0decf..aaa7c48d937 100644
--- a/library/ZendAfi/Controller/Action/Helper/Notify.php
+++ b/library/ZendAfi/Controller/Action/Helper/Notify.php
@@ -22,15 +22,13 @@
 class ZendAfi_Controller_Action_Helper_Notify extends Zend_Controller_Action_Helper_Abstract {
   protected $_options = [];
 
-  /**
-   * [[file:~/public_html/afi-opac3/library/Class/ScriptLoader.php::public%20function%20showNotifications()%20{][voir Class_ScriptLoader::showNotifications]]
-   */
   public function notify($message, $options=[]) {
     $this->getActionController()
          ->getHelper('flashMessenger')
          ->addNotification($message, array_merge($this->_options, $options));
   }
 
+
   public function direct($message, $options=[]) {
     $this->notify($message, $options);
   }
diff --git a/tests/application/modules/opac/controllers/AuthControllerTest.php b/tests/application/modules/opac/controllers/AuthControllerTest.php
index bdf03fe12b1..da62eb31f7f 100644
--- a/tests/application/modules/opac/controllers/AuthControllerTest.php
+++ b/tests/application/modules/opac/controllers/AuthControllerTest.php
@@ -2745,3 +2745,61 @@ class AuthControllerPostLoginWithDifferentIdIntBibTest
     $this->assertEquals(56, $user->getIdIntBib());
   }
 }
+
+
+
+
+// see http://forge.afi-sa.fr/issues/100763
+class AuthControllerPostLoginOpsysWithExceptionTest extends AbstractControllerTestCase {
+
+  protected $_storm_default_to_volatile = true;
+
+  public function setUp() {
+    parent::setUp();
+
+    ZendAfi_Auth::getInstance()->clearIdentity();
+
+    $this->fixture('Class_IntBib',
+                   ['id' => 44 ,
+                    'comm_sigb' => Class_IntBib::COM_OPSYS,
+                    'comm_params' => serialize(['url_serveur' => ''])]);
+
+    $service = $this->mock()
+                    ->whenCalled('providesAuthentication')->answers(true)
+                    ->whenCalled('isConnected')->answers(true)
+                    ->whenCalled('providesPagedLoans')->answers(false)
+
+                    ->whenCalled('getEmprunteur')
+                    ->answers(Class_WebService_SIGB_Emprunteur::newInstance(2233, 'harlock')->beValid())
+
+                    ->whenCalled('getEmpruntsOf')
+                    ->willDo(function()
+                             {
+                               throw new SoapFault('1512', 'Character reference "&#x1E" is an invalid XML character');
+                             });
+
+    Class_WebService_SIGB_Opsys::setService($service);
+
+    $this->postDispatch('/opac/auth/login',
+                        ['username' => 'harlock',
+                         'password' => 'arcadia4ever']);
+  }
+
+
+  public function tearDown() {
+    Class_WebService_SIGB_Opsys::setService(null);
+    parent::tearDown();
+  }
+
+
+  /** @test */
+  public function shouldBeLogged() {
+    $this->assertNotNull(ZendAfi_Auth::getInstance()->getIdentity());
+  }
+
+
+  /** @test */
+  public function shouldNotifyErrorWithExceptionMessage() {
+    $this->assertFlashMessengerContentContains('Character reference');
+  }
+}
diff --git a/tests/library/Class/WebService/SIGB/OpsysServiceTest.php b/tests/library/Class/WebService/SIGB/OpsysServiceTest.php
index 2c2d383a7d5..7812c81afdc 100644
--- a/tests/library/Class/WebService/SIGB/OpsysServiceTest.php
+++ b/tests/library/Class/WebService/SIGB/OpsysServiceTest.php
@@ -182,7 +182,9 @@ class OpsysServiceTestAutoConnect extends ModelTestCase {
 }
 
 
-class Class_System_OpsysServiceFactoryTestUrls extends ModelTestCase {
+
+
+class OpsysServiceFactoryTestUrls extends ModelTestCase {
   public function setUp() {
     $this->factory = new Class_WebService_SIGB_Opsys_ServiceFactory();
   }
@@ -217,7 +219,7 @@ class Class_System_OpsysServiceFactoryTestUrls extends ModelTestCase {
 
 
 
-class Class_WebService_SIGB_OpsysServiceTestProxy extends ModelTestCase {
+class OpsysServiceTestProxy extends ModelTestCase {
 
   protected $_storm_default_to_volatile = true,
     $factory,
@@ -714,7 +716,7 @@ class OpsysServiceGetExemplaireFromCacheTestDisponibilite extends OpsysServiceWi
     }
 
     $recuperer_notice_res = $this->createMock('RecupererNoticeResponse',
-                                           array('createNotice'));
+                                              array('createNotice'));
     $recuperer_notice_res
       ->expects($this->once())
       ->method('createNotice')
@@ -1734,4 +1736,30 @@ class OpsysServiceDisconnectTest extends OpsysServiceWithSessionTestCase {
   public function serveurSessionNomServeurShouldBeInternet() {
     $this->assertEquals('INTERNET', $this->_fermer_session->Param->ListeServeurs[0]->NomServeur);
   }
-}
\ No newline at end of file
+}
+
+
+
+
+// see http://forge.afi-sa.fr/issues/100763
+class OpsysServiceGetEmpruntsOfWithInvalidXmlTest extends ModelTestCase {
+  /**
+   * @test
+   * @expectedException SoapFault
+   */
+  public function shouldThrowException() {
+    $client = $this->mock()
+                   ->whenCalled('OuvrirSession')
+                   ->answers($this->mock()
+                             ->whenCalled('getGUID')
+                             ->answers('1234'))
+
+                   ->whenCalled('EmprListerEntite')
+                   ->willDo(function()
+                            {
+                              throw new SoapFault('1512', 'Character reference "&#x1E" is an invalid XML character');
+                            });
+
+    (new Class_WebService_SIGB_Opsys_Service($client))->getEmpruntsOf($this->mock());
+  }
+}
-- 
GitLab