From 9a2ef01cfc0d83e8746370fd55816e206e747456 Mon Sep 17 00:00:00 2001 From: Lacey Sanderson Date: Mon, 17 Oct 2022 13:46:37 -0600 Subject: [PATCH 1/4] Add test specifically table-prefixing failing currently in tests. --- .../Database/ChadoConnectionTest.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php diff --git a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php new file mode 100644 index 00000000..da72f357 --- /dev/null +++ b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php @@ -0,0 +1,62 @@ +query() + * or you can use the various query builders: CONNECTION->select(), + * CONNECTION->update(), CONNECTION->merge(), CONNECTION->upsert(), CONNECTION->delete(). + * + * This test is focusing on CONNECTION->query() since a code analysis shows + * that the other options are simply preparing a query and then executing it + * using CONNECTION->query(). + * + * That said, at some point we may want to add additional tests to show that + * the query builders are building queries appropriately but because these + * are Drupal functionalities and our differences come during execution + * and not at the query building stage, we are currently going to assume that + * the Drupal testing is sufficient for the query builders. + */ + public function testDefaultTablePrefixing() { + + // Open a Connection to the default Tripal DBX managed Chado schema. + $connection = \Drupal::service('tripal_chado.database'); + $chado_1_prefix = $connection->getSchemaName(); + + // Create a situation where we should be using the core chado schema for our query. + $query = $connection->query("SELECT name, uniquename FROM {1:feature} LIMIT 1"); + $sqlStatement = $query->getQueryString(); + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "The sql statement does not have the table prefix we expect."); + + // Test the API realizes that chado is the default schema for this query. + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + $sqlStatement = $query->getQueryString(); + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "Unable to determine that chado is the default schema because the sql statement does not have the table prefix we expect."); + } +} From 4350cd7a21026eadc1bf199295003b40c0438880 Mon Sep 17 00:00:00 2001 From: Lacey Sanderson Date: Mon, 17 Oct 2022 13:57:35 -0600 Subject: [PATCH 2/4] Fix whitespace. --- .../Database/ChadoConnectionTest.php | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php index da72f357..6d66998e 100644 --- a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php +++ b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php @@ -12,51 +12,51 @@ */ class ChadoConnectionTest extends ChadoTestBrowserBase { - /** + /** * {@inheritdoc} */ protected static $modules = ['tripal', 'tripal_chado']; - /** + /** * Tests table prefixing by the ChadoConnection + TripalDbxConnection classes. - * - * NOTE: - * In Drupal you can execute queries directly using CONNECTION->query() - * or you can use the various query builders: CONNECTION->select(), - * CONNECTION->update(), CONNECTION->merge(), CONNECTION->upsert(), CONNECTION->delete(). - * - * This test is focusing on CONNECTION->query() since a code analysis shows - * that the other options are simply preparing a query and then executing it - * using CONNECTION->query(). - * - * That said, at some point we may want to add additional tests to show that - * the query builders are building queries appropriately but because these - * are Drupal functionalities and our differences come during execution - * and not at the query building stage, we are currently going to assume that - * the Drupal testing is sufficient for the query builders. + * + * NOTE: + * In Drupal you can execute queries directly using CONNECTION->query() + * or you can use the various query builders: CONNECTION->select(), + * CONNECTION->update(), CONNECTION->merge(), CONNECTION->upsert(), CONNECTION->delete(). + * + * This test is focusing on CONNECTION->query() since a code analysis shows + * that the other options are simply preparing a query and then executing it + * using CONNECTION->query(). + * + * That said, at some point we may want to add additional tests to show that + * the query builders are building queries appropriately but because these + * are Drupal functionalities and our differences come during execution + * and not at the query building stage, we are currently going to assume that + * the Drupal testing is sufficient for the query builders. */ public function testDefaultTablePrefixing() { - // Open a Connection to the default Tripal DBX managed Chado schema. - $connection = \Drupal::service('tripal_chado.database'); - $chado_1_prefix = $connection->getSchemaName(); + // Open a Connection to the default Tripal DBX managed Chado schema. + $connection = \Drupal::service('tripal_chado.database'); + $chado_1_prefix = $connection->getSchemaName(); - // Create a situation where we should be using the core chado schema for our query. - $query = $connection->query("SELECT name, uniquename FROM {1:feature} LIMIT 1"); - $sqlStatement = $query->getQueryString(); - // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not - // necessary and could be interchanged by Drupal, we use the following pattern. - $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); - $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, - "The sql statement does not have the table prefix we expect."); + // Create a situation where we should be using the core chado schema for our query. + $query = $connection->query("SELECT name, uniquename FROM {1:feature} LIMIT 1"); + $sqlStatement = $query->getQueryString(); + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "The sql statement does not have the table prefix we expect."); - // Test the API realizes that chado is the default schema for this query. - $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); - $sqlStatement = $query->getQueryString(); - // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not - // necessary and could be interchanged by Drupal, we use the following pattern. - $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); - $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, - "Unable to determine that chado is the default schema because the sql statement does not have the table prefix we expect."); - } + // Test the API realizes that chado is the default schema for this query. + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + $sqlStatement = $query->getQueryString(); + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "Unable to determine that chado is the default schema because the sql statement does not have the table prefix we expect."); + } } From 252ab03924cc92e53480c680eced2441b3c55dba Mon Sep 17 00:00:00 2001 From: Lacey Sanderson Date: Fri, 21 Oct 2022 09:35:03 -0600 Subject: [PATCH 3/4] Show useTripalDbxSchemaFor() works as expected. --- .../Database/ChadoConnectionTest.php | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php index 6d66998e..0ee2b184 100644 --- a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php +++ b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php @@ -48,15 +48,31 @@ public function testDefaultTablePrefixing() { // necessary and could be interchanged by Drupal, we use the following pattern. $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, - "The sql statement does not have the table prefix we expect."); + "The sql statement does not have the table prefix we expect."); // Test the API realizes that chado is the default schema for this query. - $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); - $sqlStatement = $query->getQueryString(); + // We expect this to fail as the default database is chado unless Tripal DBX + // is told otherwise. + // NOTE: we use try/catch here so we can continue with our testing. + // When using expectException the execution of all other assertions is skipped. + try { + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + } catch (\Drupal\Core\Database\DatabaseExceptionWrapper $e) { + $this->assertTrue(TRUE, "We expect to have an exception thrown when TripalDBX incorrectly assumes the feature table is in Drupal, which it's not."); + } + + // Now we want to tell Tripal DBX that the default schema for this query should be chado. + $connection->useTripalDbxSchemaFor(\Drupal\Tests\tripal_chado\Functional\ChadoConnectionTest::class); + try { + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + } catch (\Drupal\Core\Database\DatabaseExceptionWrapper $e) { + $this->assertTrue(FALSE, "Now TripalDBX should know that chado is the default schema for this test class and it should not throw an exception."); + } // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not // necessary and could be interchanged by Drupal, we use the following pattern. + $sqlStatement = $query->getQueryString(); $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, - "Unable to determine that chado is the default schema because the sql statement does not have the table prefix we expect."); + "The sql statement does not have the table prefix we expect."); } } From b06f876618a3a083e3c3afbd0be56e57581aa26d Mon Sep 17 00:00:00 2001 From: Lacey Sanderson Date: Fri, 21 Oct 2022 12:47:56 -0600 Subject: [PATCH 4/4] Support inheritance in TripalDBXConnection->shouldUseTripalDbxSchema() --- tripal/src/TripalDBX/TripalDbxConnection.php | 28 +++++++++++++-- .../Database/ChadoConnectionTest.php | 34 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/tripal/src/TripalDBX/TripalDbxConnection.php b/tripal/src/TripalDBX/TripalDbxConnection.php index be320a41..69101886 100644 --- a/tripal/src/TripalDBX/TripalDbxConnection.php +++ b/tripal/src/TripalDBX/TripalDbxConnection.php @@ -992,6 +992,11 @@ protected function setPrefix($prefix) { */ protected function shouldUseTripalDbxSchema() :bool { $should = FALSE; + + // Check the class/object who is using Tripal DBX: + // We do this using the backtrace functionality with the assumption that + // the class at the deepest level of the backtrace is the one to check. + // // We start at 2 because this protected method can only be called at level 1 // from a local class method so we can skip level 1. $bt_level = 2; @@ -1005,12 +1010,29 @@ protected function shouldUseTripalDbxSchema() :bool { ++$bt_level; $calling_class = $backtrace[$bt_level]['class'] ?? ''; $calling_object = $backtrace[$bt_level]['object'] ?? FALSE; + } - if (!empty($this->classesUsingTripalDbx[$calling_class]) - || (in_array($calling_object, $this->objectsUsingTripalDbx)) - ) { + if (!empty($this->classesUsingTripalDbx[$calling_class])) { + $should = TRUE; + } + elseif (in_array($calling_object, $this->objectsUsingTripalDbx)) { $should = TRUE; } + + // Check all parents of the class who is using Tripal DBX: + // This allows for APIs to be added to the whitelist and all children class + // implementations to then automatically use the Tripal DBX managed schema. + $class = new \ReflectionClass($calling_class); + $inheritance_level = 0; + while ($parent = $class->getParentClass()) { + $inheritance_level++; + $parent_class = $parent->getName(); + if (!empty($this->classesUsingTripalDbx[$parent_class])) { + $should = TRUE; + } + $class = $parent; + } + return $should; } diff --git a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php index 0ee2b184..235075fa 100644 --- a/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php +++ b/tripal_chado/tests/src/Functional/Database/ChadoConnectionTest.php @@ -62,6 +62,25 @@ public function testDefaultTablePrefixing() { } // Now we want to tell Tripal DBX that the default schema for this query should be chado. + // Using useTripalDbxSchemaFor(): + //--------------------------------- + // PARENT CLASS: Let's check if it works when a parent class is white listed. + $connection = \Drupal::service('tripal_chado.database'); + $connection->useTripalDbxSchemaFor(\Drupal\Tests\tripal_chado\Functional\ChadoTestBrowserBase::class); + try { + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + } catch (\Drupal\Core\Database\DatabaseExceptionWrapper $e) { + $this->assertTrue(FALSE, "Now TripalDBX should know that chado is the default schema for this test class and it should not throw an exception."); + } + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $sqlStatement = $query->getQueryString(); + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "The sql statement does not have the table prefix we expect."); + + // CURRENT CLASS: Let's test it works when the current class is whitelisted + $connection = \Drupal::service('tripal_chado.database'); $connection->useTripalDbxSchemaFor(\Drupal\Tests\tripal_chado\Functional\ChadoConnectionTest::class); try { $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); @@ -74,5 +93,20 @@ public function testDefaultTablePrefixing() { $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, "The sql statement does not have the table prefix we expect."); + + // CURRENT OBJECT: Let's test it works when the current class is whitelisted + $connection = \Drupal::service('tripal_chado.database'); + $connection->useTripalDbxSchemaFor($this); + try { + $query = $connection->query("SELECT name, uniquename FROM {feature} LIMIT 1"); + } catch (\Drupal\Core\Database\DatabaseExceptionWrapper $e) { + $this->assertTrue(FALSE, "Now TripalDBX should know that chado is the default schema for this test class and it should not throw an exception."); + } + // We expect: "SCHEMAPREFIX"."feature" but since the quotes are not + // necessary and could be interchanged by Drupal, we use the following pattern. + $sqlStatement = $query->getQueryString(); + $we_expect_pattern = str_replace('SCHEMAPREFIX', $chado_1_prefix, '/["\']+SCHEMAPREFIX["\']+\.["\']+feature["\']+/'); + $this->assertMatchesRegularExpression($we_expect_pattern, $sqlStatement, + "The sql statement does not have the table prefix we expect."); } }