Code review

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
-

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;
 }

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;
 }
 
 /**

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 {}

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(), $e->getCode(), $e);
     }
 
     $stop_time = explode(' ', microtime());
@@ -113,8 +113,7 @@ protected function sendRequest($faust_number) {
           '%response' => print_r($response, TRUE),
           '%time' => round($time, 3),
         ),
-        WATCHDOG_DEBUG,
-        $link = NULL
+        WATCHDOG_DEBUG
       );
     }