altered the chado_delete_mview to be more consistent for error handling, fixed unit testing for mviews

tests/tripal_chado/api/TripalChadoMViewsAPITest.php

@@ -76,7 +76,6 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
     $mview_sql = $this->example_mviews[$mview_name]['sql'];
     $mview_schema = $this->example_mviews[$mview_name]['schema'];
     $mview_comment = $this->example_mviews[$mview_name]['comment'];
-    
     $success = chado_add_mview($mview_name, $mview_module, $mview_schema, $mview_sql, $mview_comment, FALSE);
     $this->assertTrue($success, "Failed to create materialized view: $mview_name");  
     
@@ -106,6 +105,8 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
     // Make sure the table exists.
     $this->assertTrue(chado_table_exists($mview_name),
       "Materialized view, $mview_name, was added to the tripal_mviews table but the table was not created.");
+    
+    $this->reset_tables();
   }
   /**
    * Test deletion of a materialized view.
@@ -113,19 +114,51 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_delete_mview() {
-    // TODO: this is currently a stub for a test function that neds
-    // implementation. For now it returns true to get past unit testing.
-    $this->assertTrue(true);
+
+    // Make sure the mview is present.
+    $mview_name = 'analysis_organism_test';
+    $mview_module = $this->example_mviews[$mview_name]['module'];
+    $mview_sql = $this->example_mviews[$mview_name]['sql'];
+    $mview_schema = $this->example_mviews[$mview_name]['schema'];
+    $mview_comment = $this->example_mviews[$mview_name]['comment'];    
+    $success = chado_add_mview($mview_name, $mview_module, $mview_schema, $mview_sql, $mview_comment, FALSE);
+    $this->assertTrue($success, "Failed to create materialized view: $mview_name");
+    
+    // Get the mview_id.
+    $mview = db_select('tripal_mviews', 'tm')
+      ->fields('tm')
+      ->condition('name', $mview_name)
+      ->execute()
+      ->fetchObject();
+    $this->assertTrue(is_object($mview),
+      "Failed to find the materialized view, $mview_name, in the tripal_mviews table");
+    $mview_id = $mview->mview_id;
+    
+    // Now run the API function to delete it.
+    $success = chado_delete_mview($mview_id);
+    $this->assertTrue($success, 
+      "Materialized view, $mview_name, could not be deleted.");
+
+    $this->reset_tables();
+    
+    // Make sure the table is gone.
+    $this->assertFalse(chado_table_exists($mview_name),
+      "Materialized view, $mview_name, table failed to be removed after deletion.");
+    
   }
+  
   /**
    * Test modifications to a materialized view
    *
    * @group api
    */
   public function test_chado_edit_mview() {
+        
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
+    
+    $this->reset_tables();
   }
   /**
    * Test adding a Tripal Job to re-populate a materialized view
@@ -133,9 +166,12 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_refresh_mview() {
+    
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
+    
+    $this->reset_tables();
   }
   /**
    * Test re-populating a materialized view.
@@ -143,9 +179,12 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_populate_mview() {
+   
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
+    
+    $this->reset_tables();
   }
   /**
    * Test modifications to a materialized view
@@ -153,9 +192,12 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_get_mview_id() {
+    
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
+    
+    $this->reset_tables();
   }
   /**
    * Test retrieving names of the materialized views.
@@ -163,6 +205,8 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_get_mview_table_names() {
+    $this->reset_tables();
+    
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
@@ -173,9 +217,12 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
    * @group api
    */
   public function test_chado_get_mviews() {
+   
     // TODO: this is currently a stub for a test function that neds
     // implementation. For now it returns true to get past unit testing.
     $this->assertTrue(true);
+    
+    $this->reset_tables();
   }
   
   /**
@@ -193,7 +240,6 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
     $mview_sql = $this->example_mviews[$mview_name]['sql'];
     $mview_schema = $this->example_mviews[$mview_name]['schema'];
     $mview_comment = $this->example_mviews[$mview_name]['comment'];
-    
     $success = chado_add_mview($mview_name, $mview_module, $mview_schema, $mview_sql, $mview_comment, FALSE);
     $this->assertTrue($success, "Failed to create materialized view: $mview_name");
         
@@ -209,7 +255,26 @@ class TripalChadoMViewsAPITest extends TripalTestCase {
     $success = chado_add_mview($mview_name, $mview_module, $mview_schema, $mview_sql, $mview_comment, FALSE);
     $this->assertTrue($success, "Failed to re-create materialized view: $mview_name");
     
+    // Now make sure the mview table exists.
+    $this->reset_tables();
     $this->assertTrue(chado_table_exists($mview_name),
-      "Manually removing a materialized views throws off the chado_add_mview function when the mview is re-added. See Issue #322");    
+      "Manually removing a materialized views throws off the chado_add_mview function when the mview is re-added. See Issue #322");
+    
+    $this->reset_tables();
+  }
+  
+  /**
+   * The chado_table_exists() function mantains a global variable to keep track
+   * of which tables exist.  If the table exists then it stores that info in
+   * the global variable. This is because lots of queries perform a check to
+   * see if the tables exist and that can have a major performance hit.
+   * 
+   * Because we are creating and dropping Chado tables in our tests it throws 
+   * off the array and we need to reset it. Normally this isn't a problem 
+   * because this would get reset on every page load anyone.  For testing it 
+   * doesn't.
+   */
+  public function reset_tables() {
+    $GLOBALS["chado_tables"] = [];
   }
 }

tripal_chado/api/tripal_chado.mviews.api.inc

@@ -100,17 +100,17 @@ function chado_add_mview($name, $modulename, $mv_schema, $query, $comment = NULL
       $transaction->rollback();
       watchdog_exception('tripal_chado', $e);
       $error = _drupal_decode_exception($e);
-      drupal_set_message(t("Could not create the materialized view %table_name: %message.",
-      array('%table_name' => $name, '%message' => $error['!message'])), 'error');
+      tripal_report_error('tripal_chado', TRIPAL_ERROR, 
+        "Could not create the materialized view %table_name: %message.",
+        array('%table_name' => $name, '%message' => $error['!message']));
       return FALSE;
     }
     drupal_set_message(t("Materialized view '%name' created", array('%name' => $name)));
-    return TRUE;
   }
   else {
     drupal_set_message(t("Materialized view, $name, already exists. Skipping creation.", array('%name' => $name)));
-    return FALSE;
   }
+  return TRUE;
 }
 
 /**
@@ -273,7 +273,9 @@ function chado_refresh_mview($mview_id) {
   global $user;
 
   if (!$mview_id) {
-    return '';
+    tripal_report_error('tripal_chado', TRIPAL_ERROR,
+      'Must provide an mview_id when refreshing an mview.', array());
+    return FALSE;
   }
 
   // Get this mview details.
@@ -318,39 +320,62 @@ function chado_get_mviews() {
  * @param $mview_id
  *   The unique ID of the materialized view for the action to be performed on.
  *
+ * @return
+ *   TRUE if the deletion was a success, FALSE on error.
+ *   
  * @ingroup tripal_mviews_api
  */
 function chado_delete_mview($mview_id) {
   global $user;
 
   if (!$mview_id) {
-    return '';
+    tripal_report_error('tripal_chado', TRIPAL_ERROR,
+      'Must provide an mview_id when deleting an mview.', array());
+    return FALSE;
   }
 
-  // Get this mview details.
-  $sql = "SELECT * FROM {tripal_mviews} WHERE mview_id = :mview_id";
-  $results = db_query($sql, array(':mview_id' => $mview_id));
-  $mview = $results->fetchObject();
-
-  // If op is to delete then do so.
-  // Remove the mview from the tripal_mviews table.
-  $sql = "DELETE FROM {tripal_mviews} WHERE mview_id = $mview_id";
-  db_query($sql);
-
-  // Does the table already exist?
-  $mview_exists = chado_table_exists($mview->mv_table);
-
-  // Drop the table from chado if it exists.
-  if ($mview_exists) {
-    $sql = "DROP TABLE {" . $mview->mv_table . "}";
-    $success = chado_query($sql);
-    if ($success) {
-      drupal_set_message(t("Materialized view, %name, deleted.", array('%name' => $mview->name)));
+  try {
+    $transaction = db_transaction();
+    
+    // Get this mview details.
+    $sql = "SELECT * FROM {tripal_mviews} WHERE mview_id = :mview_id";
+    $results = db_query($sql, array(':mview_id' => $mview_id));
+    $mview = $results->fetchObject();
+  
+    // If op is to delete then do so.
+    // Remove the mview from the tripal_mviews table.
+    $sql = "DELETE FROM {tripal_mviews} WHERE mview_id = $mview_id";
+    db_query($sql);
+  
+    // Does the table already exist?
+    $mview_exists = chado_table_exists($mview->mv_table);
+  
+    // Drop the table from chado if it exists.
+    if ($mview_exists) {
+      $sql = "DROP TABLE {" . $mview->mv_table . "}";
+      $success = chado_query($sql);
+      if ($success) {
+        drupal_set_message(t("Materialized view, %name, deleted.", array('%name' => $mview->name)));
+        return TRUE;
+      }
+      else {
+        drupal_set_message(t("Problem deleting materialized view, %name.", array('%name' => $mview->name)), 'error');
+        return FALSE;
+      }
     }
     else {
-      drupal_set_message(t("Problem deleting materialized view, %name.", array('%name' => $mview->name)), 'error');
+      return TRUE;
     }
   }
+  catch (Exception $e) {
+    $transaction->rollback();
+    watchdog_exception('tripal_chado', $e);
+    $error = _drupal_decode_exception($e);
+    tripal_report_error('tripal_chado', TRIPAL_ERROR,
+      "Could not delete the materialized view %table_name: %message.",
+      array('%table_name' => $name, '%message' => $error['!message']));
+  }
+  return FALSE;
 }
 
 /**