Merge pull request #404 from tripal/322-Tv3-mviews

Fix for issue #322 -- Re-adding an mview that was manually deleted.

.gitignore

@@ -2,3 +2,6 @@
 .idea/
 vendor/
 tests/.env
+.buildpath
+.project
+.settings/

composer.json

@@ -1,8 +1,8 @@
 {
   "require-dev": {
-    "doctrine/instantiator": "1.0.*"
+    "doctrine/instantiator": "1.0.*",
+    "statonlab/tripal-test-suite": "^1.1"
   },
   "require": {
-    "statonlab/tripal-test-suite": "1.0.*"
   }
 }

composer.lock

@@ -1,11 +1,12 @@
 {
     "_readme": [
         "This file locks the dependencies of your project to a known state",
-        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
+        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
         "This file is @generated automatically"
     ],
-    "content-hash": "6b7fddf464c4d4d28fc77f805792807d",
-    "packages": [
+    "content-hash": "c8b425537daabed28cd865fb02a05684",
+    "packages": [],
+    "packages-dev": [
         {
             "name": "doctrine/instantiator",
             "version": "1.0.5",
@@ -1653,16 +1654,16 @@
         },
         {
             "name": "statonlab/tripal-test-suite",
-            "version": "1.0.2",
+            "version": "1.1.0",
             "source": {
                 "type": "git",
                 "url": "https://github.com/statonlab/TripalTestSuite.git",
-                "reference": "2d09cd7565a82f16cc4bba7411bc32e2553cb92c"
+                "reference": "80b60a14b80f00d164ae5ba8511080d725526d38"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/statonlab/TripalTestSuite/zipball/2d09cd7565a82f16cc4bba7411bc32e2553cb92c",
-                "reference": "2d09cd7565a82f16cc4bba7411bc32e2553cb92c",
+                "url": "https://api.github.com/repos/statonlab/TripalTestSuite/zipball/80b60a14b80f00d164ae5ba8511080d725526d38",
+                "reference": "80b60a14b80f00d164ae5ba8511080d725526d38",
                 "shasum": ""
             },
             "require": {
@@ -1697,7 +1698,7 @@
                     "email": "bcondon@utk.edu"
                 }
             ],
-            "time": "2018-05-02T15:51:28+00:00"
+            "time": "2018-05-25T13:17:31+00:00"
         },
         {
             "name": "symfony/console",
@@ -1917,7 +1918,6 @@
             "time": "2018-01-29T19:49:41+00:00"
         }
     ],
-    "packages-dev": [],
     "aliases": [],
     "minimum-stability": "stable",
     "stability-flags": [],

tests/tripal_chado/api/TripalChadoCustomTablesAPITest.php

@@ -7,7 +7,6 @@ use StatonLab\TripalTestSuite\TripalTestCase;
 class TripalChadoCustomTablesAPITest extends TripalTestCase {
   // Uncomment to auto start and rollback db transactions per test method.
   // use DBTransaction;
-
   /**
    * Basic test example.
    * Tests must begin with the word "test".

tests/tripal_chado/api/TripalChadoMViewsAPITest.php

@@ -0,0 +1,280 @@
+<?php
+namespace Tests\tripal_chado\api;
+
+use StatonLab\TripalTestSuite\DBTransaction;
+use StatonLab\TripalTestSuite\TripalTestCase;
+
+class TripalChadoMViewsAPITest extends TripalTestCase {
+  
+  // Use a transaction to roll back changes after every test.
+  use DBTransaction;
+  
+  // This variable holds example materialized views that can be used
+  // by the unit tests below.
+  private $example_mviews = [
+    'analysis_organism_test' => [
+      'schema' => [
+        'table' => 'analysis_organism_test',
+        'description' => 'This view is for associating an organism (via it\'s associated features) to an analysis.',
+        'fields' => [
+          'analysis_id' => [
+            'size' => 'big',
+            'type' => 'int',
+            'not null' => true,
+          ],
+          'organism_id' => [
+            'size' => 'big',
+            'type' => 'int',
+            'not null' => true,
+          ],
+        ],
+        'indexes' => [
+          'networkmod_qtl_indx0' => [
+            0 => 'analysis_id',
+          ],
+          'networkmod_qtl_indx1' => [
+            0 => 'organism_id',
+          ],
+        ],
+        'foreign keys' => [
+          'analysis' => [
+            'table' => 'analysis',
+            'columns' => [
+              'analysis_id' => 'analysis_id',
+            ],
+          ],
+          'organism' => [
+            'table' => 'organism',
+            'columns' => [
+              'organism_id' => 'organism_id',
+            ],
+          ],
+        ],
+      ],
+      'sql' => "
+        SELECT DISTINCT A.analysis_id, O.organism_id
+        FROM analysis A
+          INNER JOIN analysisfeature AF ON A.analysis_id = AF.analysis_id
+          INNER JOIN feature F          ON AF.feature_id = F.feature_id
+          INNER JOIN organism O         ON O.organism_id = F.organism_id
+      ",
+      'comment' => 'This view is for associating an organism (via it\'s associated features) to an analysis.',
+      'module' => 'tripal_chado',
+    ],
+  ];
+  
+  /**
+   * Test creation of a new materialized view.
+   *
+   * @group api
+   */
+  public function test_chado_add_mview() {
+    
+    // Add the analysis_organism mview.
+    $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");  
+    
+    // Make sure that the entry is now there.
+    $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");
+    
+    // Make sure that all of the fields exist and were properly added.
+    $this->assertTrue($mview->modulename == $mview_module,
+      "Failed to create a proper materialized the modulename field is incorrect: '$mview_module' != '$mview->modulename'");
+    $this->assertTrue($mview->mv_table == $mview_name,
+      "Failed to create a proper materialized the mv_table field does not match input.");
+    $this->assertTrue($mview->query == $mview_sql,
+      "Failed to create a proper materialized the query field does not match input.");
+    $this->assertTrue($mview->comment == $mview_comment,
+      "Failed to create a proper materialized the comment field does not match input.");
+    $this->assertNULL($mview->status,
+      "Failed to create a proper materialized the status field should be NULL.");
+    $this->assertNULL($mview->last_update,
+      "Failed to create a proper materialized the last_update field should be NULL.");
+    
+    // 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.
+   *
+   * @group api
+   */
+  public function test_chado_delete_mview() {
+
+    // 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
+   *
+   * @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.
+   *
+   * @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
+   *
+   * @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.
+   *
+   * @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);
+  }
+  /**
+   * Test retrieving all materialized view objects.
+   *
+   * @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();
+  }
+  
+  /**
+   * Issue 322 reported the problem of re-adding a materialized view after
+   * the actual table had been manually removed outside of Tripal.  The
+   * function reported errors.
+   *
+   * @ticket 322
+   */
+  public function test_re_adding_deleted_mview_issue_322() {
+    
+    // Add the analysis_organism mview.
+    $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");
+        
+    // Now simulate manual deletion of the table outside of the API.
+    chado_query('DROP TABLE {' . $mview_name . '}');
+    
+    // Make sure the table no longer exists.
+    $this->assertFalse(chado_table_exists($mview_name), 
+      "Failed to manually remove the materialized view, cannot complete the test.");
+    
+    // Now try to read the mview. Previously, the behavior was the the mview
+    // table would not be created because Tripal thinks it's already there.
+    $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");
+    
+    $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

@@ -16,12 +16,8 @@
  */
 
 /**
- * Add a materialized view to the chado database to help speed data access. This
- * function supports the older style where postgres column specifications
- * are provided using the $mv_table, $mv_specs and $indexed variables. It also
- * supports the newer preferred method where the materialized view is described
- * using the Drupal Schema API array.
- *
+ * Add a materialized view to the chado database. 
+ * 
  * @param $name
  *   The name of the materialized view.
  * @param $modulename
@@ -40,6 +36,9 @@
  *   admin pages. However, when called by Drush we don't want to redirect. This
  *   parameter allows this to be used as a true API function.
  *
+ * @return 
+ *   TRUE if the materialized view was successfully added, FALSE otherwise.
+ *   
  * @ingroup tripal_mviews_api
  */
 function chado_add_mview($name, $modulename, $mv_schema, $query, $comment = NULL, $redirect = TRUE) {
@@ -47,16 +46,27 @@ function chado_add_mview($name, $modulename, $mv_schema, $query, $comment = NULL
   if (!array_key_exists('table', $mv_schema)) {
      tripal_report_error('tripal_chado', TRIPAL_ERROR,
        'Must have a table name when creating an mview.', array());
-     return NULL;
+     return FALSE;
   }
 
   $mv_table = $mv_schema['table'];
-
+ 
   // See if the mv_table name already exsists.
-  $mview_id = db_query(
-    'SELECT mview_id FROM {tripal_mviews} WHERE name = :name',
-    array(':name' => $name))->fetchField();
-
+  $mview_id = db_select('tripal_mviews', 'tm')
+    ->fields('tm', array('mview_id'))
+    ->condition('name', $name)
+    ->execute()
+    ->fetchField();
+
+  // Check that the materialized view actually exists and if not,
+  // remove the entry from tripal_mviews.
+  if ($mview_id AND !chado_table_exists($name)) {
+    db_delete('tripal_mviews')
+      ->condition('mview_id', $mview_id)
+      ->execute();
+    $mview_id = FALSE;
+  }
+  
   if(!$mview_id) {
     $transaction = db_transaction();
     try {
@@ -90,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;
 }
 
 /**
@@ -263,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.
@@ -308,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;
 }
 
 /**