From 79a9d59023fd19965ff354aa8317884d3b1268ed Mon Sep 17 00:00:00 2001 From: Kasper Garnaes Date: Sun, 10 Apr 2022 13:06:11 +0200 Subject: [PATCH 1/5] Address multiple IMS exceptions The implementation only contains a single exception class but the code refers to multiple classes ImsServiceException and ImsException. There does not seem to be a need for multiple exception classes so only use one: ImsException. Update code referring to ImsServiceException accordingly. --- ims.module | 13 ++++--------- lib/ims-client/ImsException.php | 2 +- lib/ims-client/ImsService.php | 4 ++-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/ims.module b/ims.module index 3d9dd70..c16194a 100644 --- a/ims.module +++ b/ims.module @@ -50,15 +50,10 @@ function ims_ding_provider() { */ function ims_placements(array $provider_ids) { // Call the IMS Webservice. - try { - $service = new ImsService(variable_get('ims_wsdl_url'), variable_get('ims_username'), variable_get('ims_password')); + $service = new ImsService(variable_get('ims_wsdl_url'), variable_get('ims_username'), variable_get('ims_password')); + + // Retrieve placements for multiple ting object ids. + $retrieved = $service->getByFaustNumber($provider_ids); - // Retrieve placements for multiple ting object ids. - $retrieved = $service->getByFaustNumber($provider_ids); - } - catch (ImsException $e) { - // Re-throw Ims specific exception. - throw new ImsServiceException($e->getMessage()); - } return $retrieved; } diff --git a/lib/ims-client/ImsException.php b/lib/ims-client/ImsException.php index 51411ec..e641a19 100644 --- a/lib/ims-client/ImsException.php +++ b/lib/ims-client/ImsException.php @@ -8,4 +8,4 @@ /** * Exception related to communication with the IMS SOAP Webservice. */ -class ImsServiceException extends Exception {} +class ImsException extends Exception {} diff --git a/lib/ims-client/ImsService.php b/lib/ims-client/ImsService.php index 5af9674..9ad20ac 100644 --- a/lib/ims-client/ImsService.php +++ b/lib/ims-client/ImsService.php @@ -74,7 +74,7 @@ public function getByFaustNumber($faust_numbers) { * @return object * SOAP Response object. * - * @throws ImsServiceException + * @throws ImsException */ protected function sendRequest($faust_number) { $auth_info = array( @@ -98,7 +98,7 @@ protected function sendRequest($faust_number) { } catch (Exception $e) { // Re-throw Ims specific exception. - throw new ImsServiceException($e->getMessage()); + throw new ImsException($e->getMessage()); } $stop_time = explode(' ', microtime()); From 5d6948563b98f16c8218135e5c6f836a5ae20ca0 Mon Sep 17 00:00:00 2001 From: Kasper Garnaes Date: Sun, 10 Apr 2022 13:06:55 +0200 Subject: [PATCH 2/5] Remove reference to nonexistent file ImsPlacement --- ims.info | 2 -- 1 file changed, 2 deletions(-) diff --git a/ims.info b/ims.info index fef29b8..8b8575a 100644 --- a/ims.info +++ b/ims.info @@ -6,7 +6,5 @@ core = "7.x" dependencies[] = ding_provider dependencies[] = fbs configure = admin/config/ding/ims -files[] = lib/ims-client/ImsPlacement.php files[] = lib/ims-client/ImsException.php files[] = lib/ims-client/ImsService.php - From 1e1cbee924bb7b76f564a770157ed4b812aa8741 Mon Sep 17 00:00:00 2001 From: Kasper Garnaes Date: Sun, 10 Apr 2022 13:08:20 +0200 Subject: [PATCH 3/5] Do not condition availability loading on Ims module being enabled Code located in the module should not be executed if the module is not enabled. --- inc/ims.availability.inc | 69 +++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/inc/ims.availability.inc b/inc/ims.availability.inc index 3fe1dcd..310db29 100644 --- a/inc/ims.availability.inc +++ b/inc/ims.availability.inc @@ -39,47 +39,44 @@ function ims_availability_holdings(array $provider_ids) { module_load_include('inc', 'fbs', 'includes/fbs.availability'); $fbs_results = fbs_availability_holdings($provider_ids); - // If the ims module is enabled we try to fetch ims placements. - if (module_exists('ims')) { - try { - // Fetch ims placements for all $provider_ids. - $ims_results = ims_placements($provider_ids); - } - catch (Exception $e) { - watchdog_exception('ims', $e); - // Could't get data from IMS. Just quietly return results from fbs. - return $fbs_results; - } + try { + // Fetch ims placements for all $provider_ids. + $ims_results = ims_placements($provider_ids); + } + catch (Exception $e) { + watchdog_exception('ims', $e); + // Could't get data from IMS. Just quietly return results from fbs. + return $fbs_results; + } - // Loop each faust-number. - foreach ($fbs_results as $faust => $fbs_result) { - // Ims placements for current faust. - $ims_result = $ims_results[$faust]; - // Periodicals have holdings pr. volume for each issue - // Requires extra iterations. - if ($fbs_result['is_periodical']) { - $issues = $fbs_result['issues']; - // We need to adjust holdings for each volume in each issue. - foreach ($issues as $issue_key => $volumes) { - foreach ($volumes as $volume_key => $volume) { - // Create and ajust holdings to fit in ims_placements. - $fbs_adjusted_holdings = _ims_merge_ims_placements($volume['placement'], $ims_result); - $fbs_results[$faust]['issues'][$issue_key][$volume_key]['placement'] = $fbs_adjusted_holdings; - } + // Loop each faust-number. + foreach ($fbs_results as $faust => $fbs_result) { + // Ims placements for current faust. + $ims_result = $ims_results[$faust]; + // Periodicals have holdings pr. volume for each issue + // Requires extra iterations. + if ($fbs_result['is_periodical']) { + $issues = $fbs_result['issues']; + // We need to adjust holdings for each volume in each issue. + foreach ($issues as $issue_key => $volumes) { + foreach ($volumes as $volume_key => $volume) { + // Create and ajust holdings to fit in ims_placements. + $fbs_adjusted_holdings = _ims_merge_ims_placements($volume['placement'], $ims_result); + $fbs_results[$faust]['issues'][$issue_key][$volume_key]['placement'] = $fbs_adjusted_holdings; } } - // Monography. - else { - // Adjust Fbs holdings for a monography. - $fbs_holdings = $fbs_result['holdings']; - // Create and ajust holdings to fit in ims_placements. - $fbs_adjusted_holdings = _ims_merge_ims_placements($fbs_holdings, $ims_result); - $fbs_results[$faust]['holdings'] = $fbs_adjusted_holdings; - } } - - return $fbs_results; + // Monography. + else { + // Adjust Fbs holdings for a monography. + $fbs_holdings = $fbs_result['holdings']; + // Create and ajust holdings to fit in ims_placements. + $fbs_adjusted_holdings = _ims_merge_ims_placements($fbs_holdings, $ims_result); + $fbs_results[$faust]['holdings'] = $fbs_adjusted_holdings; + } } + + return $fbs_results; } /** From 22d37437847e75cf21ef2b5836d44b1a7d506136 Mon Sep 17 00:00:00 2001 From: Kasper Garnaes Date: Sun, 10 Apr 2022 13:10:56 +0200 Subject: [PATCH 4/5] Remove watchdog link variable assignment It is unneeded and variable assignment in arguments is confusing. --- lib/ims-client/ImsService.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ims-client/ImsService.php b/lib/ims-client/ImsService.php index 9ad20ac..30dadf3 100644 --- a/lib/ims-client/ImsService.php +++ b/lib/ims-client/ImsService.php @@ -113,8 +113,7 @@ protected function sendRequest($faust_number) { '%response' => print_r($response, TRUE), '%time' => round($time, 3), ), - WATCHDOG_DEBUG, - $link = NULL + WATCHDOG_DEBUG ); } From e9108a6242ee6e05d8456b9a8a274f670357f675 Mon Sep 17 00:00:00 2001 From: Kasper Garnaes Date: Sun, 10 Apr 2022 13:24:33 +0200 Subject: [PATCH 5/5] Include previous exception when throwing Ims exceptions The inner exception (usually SoapFault) may include valuable information which is not a part of the exception message. --- lib/ims-client/ImsService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ims-client/ImsService.php b/lib/ims-client/ImsService.php index 30dadf3..b9bec70 100644 --- a/lib/ims-client/ImsService.php +++ b/lib/ims-client/ImsService.php @@ -98,7 +98,7 @@ protected function sendRequest($faust_number) { } catch (Exception $e) { // Re-throw Ims specific exception. - throw new ImsException($e->getMessage()); + throw new ImsException($e->getMessage(), $e->getCode(), $e); } $stop_time = explode(' ', microtime());